Skip to content

Commit

Permalink
Delete appext.VerboseContainer
Browse files Browse the repository at this point in the history
  • Loading branch information
bufdev committed Oct 5, 2024
1 parent c44db09 commit e105e0d
Show file tree
Hide file tree
Showing 7 changed files with 40 additions and 83 deletions.
4 changes: 2 additions & 2 deletions private/buf/bufcurl/invoker.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ type invoker struct {
// in JSON format. The given resolver is used to resolve Any messages and
// extensions that appear in the input or output. Other parameters are used
// to create a Connect client, for issuing the RPC.
func NewInvoker(container appext.Container, md protoreflect.MethodDescriptor, res protoencoding.Resolver, emitDefaults bool, httpClient connect.HTTPClient, opts []connect.ClientOption, url string, out io.Writer) Invoker {
func NewInvoker(container appext.Container, verbosePrinter verbose.Printer, md protoreflect.MethodDescriptor, res protoencoding.Resolver, emitDefaults bool, httpClient connect.HTTPClient, opts []connect.ClientOption, url string, out io.Writer) Invoker {
opts = append(opts, connect.WithCodec(protoCodec{}))
// TODO: could also provide custom compressor implementations that could give us
// optics into when request and response messages are compressed (which could be
Expand All @@ -94,7 +94,7 @@ func NewInvoker(container appext.Container, md protoreflect.MethodDescriptor, re
res: res,
emitDefaults: emitDefaults,
output: out,
printer: container.VerbosePrinter(),
printer: verbosePrinter,
errOutput: container.Stderr(),
client: connect.NewClient[dynamicpb.Message, deferredMessage](httpClient, url, opts...),
}
Expand Down
42 changes: 29 additions & 13 deletions private/buf/cmd/buf/command/curl/curl.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,9 @@ const (
outputFlagName = "output"
outputFlagShortName = "o"
emitDefaultsFlagName = "emit-defaults"

verboseFlagName = "verbose"
verboseFlagShortName = "v"
)

// NewCommand returns a new Command.
Expand Down Expand Up @@ -237,6 +240,8 @@ type flags struct {
Output string
EmitDefaults bool

Verbose bool

// so we can inquire about which flags present on command-line
// TODO: ideally we'd use cobra directly instead of having the appcmd wrapper,
// which prevents a lot of basic functionality by not exposing many cobra features
Expand Down Expand Up @@ -519,6 +524,14 @@ provided via stdin as a file descriptor set or image`,
false,
`Emit default values for JSON-encoded responses.`,
)

flagSet.BoolVarP(
&f.Verbose,
verboseFlagName,
verboseFlagShortName,
false,
"Turn on verbose mode",
)
}

func (f *flags) validate(hasURL, isSecure bool) error {
Expand Down Expand Up @@ -648,10 +661,8 @@ func (f *flags) validate(hasURL, isSecure bool) error {

func (f *flags) determineCredentials(
ctx context.Context,
container interface {
app.Container
appext.VerboseContainer
},
container app.Container,
verbosePrinter verbose.Printer,
host string,
) (string, error) {
if f.User != "" {
Expand Down Expand Up @@ -687,7 +698,7 @@ func (f *flags) determineCredentials(
if _, err := os.Stat(netrcFile); err != nil {
if errors.Is(err, os.ErrNotExist) {
// This mirrors the behavior of curl when a netrc file does not exist ¯\_(ツ)_/¯
container.VerbosePrinter().Printf("* Couldn't find host %s in the file %q; using no credentials", host, netrcFile)
verbosePrinter.Printf("* Couldn't find host %s in the file %q; using no credentials", host, netrcFile)
return "", nil
}
if !strings.Contains(err.Error(), netrcFile) {
Expand All @@ -702,7 +713,7 @@ func (f *flags) determineCredentials(
}
if machine == nil {
// no creds found for this host
container.VerbosePrinter().Printf("* Couldn't find host %s in the file %q; using no credentials", host, netrcFile)
verbosePrinter.Printf("* Couldn't find host %s in the file %q; using no credentials", host, netrcFile)
return "", nil
}
username := machine.Login()
Expand Down Expand Up @@ -868,6 +879,11 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error)
return err
}

var verbosePrinter verbose.Printer = verbose.NopPrinter
if f.Verbose {
verbosePrinter = verbose.NewPrinter(container.Stderr(), container.AppName())
}

var clientOptions []connect.ClientOption
switch f.Protocol {
case connect.ProtocolGRPC:
Expand All @@ -881,7 +897,7 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error)
// in an end-of-stream message for streaming calls. So this interceptor
// will print the trailers for streaming calls when the response stream
// is drained.
clientOptions = append(clientOptions, connect.WithInterceptors(bufcurl.TraceTrailersInterceptor(container.VerbosePrinter())))
clientOptions = append(clientOptions, connect.WithInterceptors(bufcurl.TraceTrailersInterceptor(verbosePrinter)))
}

dataSource := "(argument)"
Expand Down Expand Up @@ -910,7 +926,7 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error)
}
var basicCreds *string
if len(requestHeaders.Values("authorization")) == 0 {
creds, err := f.determineCredentials(ctx, container, host)
creds, err := f.determineCredentials(ctx, container, verbosePrinter, host)
if err != nil {
return err
}
Expand Down Expand Up @@ -950,11 +966,11 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error)
// This shouldn't be possible since we check in flags.validate, but just in case
return nil, errors.New("URL positional argument is missing")
}
roundTripper, err := makeHTTPRoundTripper(f, isSecure, bufcurl.GetAuthority(host, requestHeaders), container.VerbosePrinter())
roundTripper, err := makeHTTPRoundTripper(f, isSecure, bufcurl.GetAuthority(host, requestHeaders), verbosePrinter)
if err != nil {
return nil, err
}
return bufcurl.NewVerboseHTTPClient(roundTripper, container.VerbosePrinter()), nil
return bufcurl.NewVerboseHTTPClient(roundTripper, verbosePrinter), nil
})

output := container.Stdout()
Expand All @@ -976,7 +992,7 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error)
if basicCreds != nil {
creds = *basicCreds
} else {
if creds, err = f.determineCredentials(ctx, container, host); err != nil {
if creds, err = f.determineCredentials(ctx, container, verbosePrinter, host); err != nil {
return err
}
}
Expand All @@ -995,7 +1011,7 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error)
if err != nil {
return err
}
res, closeRes := bufcurl.NewServerReflectionResolver(ctx, transport, clientOptions, baseURL, reflectProtocol, reflectHeaders, container.VerbosePrinter())
res, closeRes := bufcurl.NewServerReflectionResolver(ctx, transport, clientOptions, baseURL, reflectProtocol, reflectHeaders, verbosePrinter)
defer closeRes()
resolvers = append(resolvers, res)
}
Expand Down Expand Up @@ -1064,7 +1080,7 @@ func run(ctx context.Context, container appext.Container, f *flags) (err error)
if err != nil {
return err
}
invoker := bufcurl.NewInvoker(container, methodDescriptor, res, f.EmitDefaults, transport, clientOptions, urlArg, output)
invoker := bufcurl.NewInvoker(container, verbosePrinter, methodDescriptor, res, f.EmitDefaults, transport, clientOptions, urlArg, output)
return invoker.Invoke(ctx, dataSource, dataReader, requestHeaders)
}
}
Expand Down
2 changes: 0 additions & 2 deletions private/buf/cmd/internal/internal.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"github.com/bufbuild/buf/private/bufpkg/bufconfig"
"github.com/bufbuild/buf/private/pkg/app"
"github.com/bufbuild/buf/private/pkg/app/appext"
"github.com/bufbuild/buf/private/pkg/verbose"
"github.com/bufbuild/buf/private/pkg/zaputil"
"github.com/bufbuild/protoplugin"
)
Expand Down Expand Up @@ -103,7 +102,6 @@ func NewAppextContainerForPluginEnv(
appContainer,
appName,
logger,
verbose.NopPrinter,
)
}

Expand Down
17 changes: 0 additions & 17 deletions private/pkg/app/appext/appext.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (

"github.com/bufbuild/buf/private/pkg/app"
"github.com/bufbuild/buf/private/pkg/encoding"
"github.com/bufbuild/buf/private/pkg/verbose"
"github.com/spf13/pflag"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
Expand Down Expand Up @@ -90,39 +89,23 @@ func NewLoggerContainer(logger *zap.Logger) LoggerContainer {
return newLoggerContainer(logger)
}

// VerboseContainer provides a verbose.Printer.
type VerboseContainer interface {
// VerboseEnabled returns true if verbose mode is enabled.
VerboseEnabled() bool
// VerbosePrinter returns a verbose.Printer to use for verbose printing.
VerbosePrinter() verbose.Printer
}

// NewVerboseContainer returns a new VerboseContainer.
func NewVerboseContainer(verbosePrinter verbose.Printer) VerboseContainer {
return newVerboseContainer(verbosePrinter)
}

// Container contains not just the base app container, but all extended containers.
type Container interface {
app.Container
NameContainer
LoggerContainer
VerboseContainer
}

// NewContainer returns a new Container.
func NewContainer(
baseContainer app.Container,
appName string,
logger *zap.Logger,
verbosePrinter verbose.Printer,
) (Container, error) {
return newContainer(
baseContainer,
appName,
logger,
verbosePrinter,
)
}

Expand Down
11 changes: 6 additions & 5 deletions private/pkg/app/appext/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"github.com/bufbuild/buf/private/pkg/app"
"github.com/bufbuild/buf/private/pkg/thread"
"github.com/bufbuild/buf/private/pkg/verbose"
"github.com/bufbuild/buf/private/pkg/zaputil"
"github.com/pkg/profile"
"github.com/spf13/pflag"
Expand All @@ -34,7 +33,6 @@ import (
type builder struct {
appName string

verbose bool
debug bool
noWarn bool
logFormat string
Expand Down Expand Up @@ -67,7 +65,6 @@ func newBuilder(appName string, options ...BuilderOption) *builder {
}

func (b *builder) BindRoot(flagSet *pflag.FlagSet) {
flagSet.BoolVarP(&b.verbose, "verbose", "v", false, "Turn on verbose mode")
flagSet.BoolVar(&b.debug, "debug", false, "Turn on debug logging")
flagSet.StringVar(&b.logFormat, "log-format", "color", "The log format [text,color,json]")
if b.defaultTimeout > 0 {
Expand All @@ -90,6 +87,11 @@ func (b *builder) BindRoot(flagSet *pflag.FlagSet) {
_ = flagSet.MarkHidden("no-warn")
flagSet.IntVar(&b.parallelism, "parallelism", 0, "Manually control the parallelism")
_ = flagSet.MarkHidden("parallelism")

// We used to have this as a global flag, so we still need to not error when it is called.
var verbose bool
flagSet.BoolVarP(&verbose, "verbose", "v", false, "")
_ = flagSet.MarkHidden("verbose")
}

func (b *builder) NewRunFunc(
Expand Down Expand Up @@ -120,8 +122,7 @@ func (b *builder) run(
defer func() {
retErr = multierr.Append(retErr, logger.Sync())
}()
verbosePrinter := verbose.NewPrinterForFlagValue(appContainer.Stderr(), b.appName, b.verbose)
container, err := newContainer(appContainer, b.appName, logger, verbosePrinter)
container, err := newContainer(appContainer, b.appName, logger)
if err != nil {
return err
}
Expand Down
10 changes: 3 additions & 7 deletions private/pkg/app/appext/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,27 @@ package appext

import (
"github.com/bufbuild/buf/private/pkg/app"
"github.com/bufbuild/buf/private/pkg/verbose"
"go.uber.org/zap"
)

type container struct {
app.Container
NameContainer
LoggerContainer
VerboseContainer
}

func newContainer(
baseContainer app.Container,
appName string,
logger *zap.Logger,
verbosePrinter verbose.Printer,
) (*container, error) {
nameContainer, err := newNameContainer(baseContainer, appName)
if err != nil {
return nil, err
}
return &container{
Container: baseContainer,
NameContainer: nameContainer,
LoggerContainer: newLoggerContainer(logger),
VerboseContainer: newVerboseContainer(verbosePrinter),
Container: baseContainer,
NameContainer: nameContainer,
LoggerContainer: newLoggerContainer(logger),
}, nil
}
37 changes: 0 additions & 37 deletions private/pkg/app/appext/verbose_container.go

This file was deleted.

0 comments on commit e105e0d

Please sign in to comment.