Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make logger and repository service client optional in api module reader #2594

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion private/buf/bufcli/bufcli.go
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,9 @@ func newModuleReaderAndCreateCacheDirs(
delegateReader := bufapimodule.NewModuleReader(
container.Logger(),
bufapimodule.NewDownloadServiceClientFactory(clientConfig),
bufapimodule.NewRepositoryServiceClientFactory(clientConfig),
bufapimodule.ModuleReaderWithDeprecationWarning(
bufapimodule.NewRepositoryServiceClientFactory(clientConfig),
),
)
storageosProvider := storageos.NewProvider(storageos.ProviderWithSymlinks())
var moduleReader bufmodule.ModuleReader
Expand Down
12 changes: 10 additions & 2 deletions private/bufpkg/bufapimodule/bufapimodule.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,28 @@ func NewRepositoryServiceClientFactory(clientConfig *connectclient.Config) Repos
func NewModuleReader(
logger *zap.Logger,
downloadClientFactory DownloadServiceClientFactory,
repositoryClientFactory RepositoryServiceClientFactory,
opts ...ModuleReaderOption,
) bufmodule.ModuleReader {
return newModuleReader(
logger,
downloadClientFactory,
repositoryClientFactory,
opts...,
)
}

// ModuleReaderOption allows configuration of a module reader.
type ModuleReaderOption func(reader *moduleReader)

// ModuleReaderWithDeprecationWarning makes the module reader print a warning
// when reading a deprecated module.
func ModuleReaderWithDeprecationWarning(
repositoryClientFactory RepositoryServiceClientFactory,
) ModuleReaderOption {
return func(reader *moduleReader) {
reader.repositoryClientFactory = repositoryClientFactory
}
}

// NewModuleResolver returns a new ModuleResolver backed by the resolve service.
func NewModuleResolver(
logger *zap.Logger,
Expand Down
17 changes: 9 additions & 8 deletions private/bufpkg/bufapimodule/module_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,20 @@ import (
)

type moduleReader struct {
logger *zap.Logger
downloadClientFactory DownloadServiceClientFactory
logger *zap.Logger
oliversun9 marked this conversation as resolved.
Show resolved Hide resolved
downloadClientFactory DownloadServiceClientFactory
// repositoryClientFactory may be nil
repositoryClientFactory RepositoryServiceClientFactory
}

func newModuleReader(
logger *zap.Logger,
downloadClientFactory DownloadServiceClientFactory,
repositoryClientFactory RepositoryServiceClientFactory,
opts ...ModuleReaderOption,
) *moduleReader {
reader := &moduleReader{
logger: logger,
downloadClientFactory: downloadClientFactory,
repositoryClientFactory: repositoryClientFactory,
logger: logger,
downloadClientFactory: downloadClientFactory,
}
for _, opt := range opts {
opt(reader)
Expand Down Expand Up @@ -80,8 +79,10 @@ func (m *moduleReader) GetModule(ctx context.Context, modulePin bufmoduleref.Mod
if err != nil {
return nil, err
}
if err := warnIfDeprecated(ctx, m.repositoryClientFactory, modulePin, m.logger); err != nil {
return nil, err
if m.repositoryClientFactory != nil {
if err := warnIfDeprecated(ctx, m.repositoryClientFactory, modulePin, m.logger); err != nil {
return nil, err
}
}
return bufmodule.NewModuleForFileSet(ctx, fileSet, identityAndCommitOpt)
}
Expand Down
3 changes: 0 additions & 3 deletions private/bufpkg/bufapimodule/module_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,6 @@ func testDownload(
moduleReader := newModuleReader(
zap.NewNop(),
mock.factory,
func(string) registryv1alpha1connect.RepositoryServiceClient {
return &nopRepositoryServiceClient{}
},
moduleReaderOpts...,
)
ctx := context.Background()
Expand Down