Add StorageConfigList to config response #8537
Conversation
N-o-Z
added
exclude-changelog
|
♻️ PR Preview 3a75969 has been successfully destroyed since this PR has been closed.
🤖 By surge-preview |
E2E Test Results - Quickstart |
E2E Test Results - DynamoDB Local - Local Block Adapter |
|
|
||
| StorageConfigList: | ||
| type: array | ||
| items: |
There was a problem hiding this comment.
In the PRD there are pagination and results here -
Assuming these are not really needed, but can you please verify this and/or change the PRD accordingly?
There was a problem hiding this comment.
Not only is it not needed - it is a problem because StorageConfigList is nested inside the Config response.
| @@ -137,7 +137,8 @@ var runCmd = &cobra.Command{ | |||
| Short: "Run lakeFS", | |||
| Run: func(cmd *cobra.Command, args []string) { | |||
| logger := logging.ContextUnavailable() | |||
| cfg := loadConfig().GetBaseConfig() | |||
| cfg := loadConfig() | |||
There was a problem hiding this comment.
Why this change is needed?
I couldn't find any other usage of cfg in this function -
Can you point to where it's used?
There was a problem hiding this comment.
cfg is the interface and should be passed to the block factory
| } | ||
|
|
||
| storageCfg := c.getStorageConfig() | ||
| // TODO (niro): Needs to be populated | ||
| storageListCfg := apigen.StorageConfigList{} |
There was a problem hiding this comment.
So should there be cfg.getStorageConfigList(), or any other usage of cfg?
| @@ -33,15 +33,15 @@ const ( | |||
| extensionValidationExcludeBody = "x-validation-exclude-body" | |||
| ) | |||
|
|
|||
| func Serve(cfg *config.BaseConfig, catalog *catalog.Catalog, middlewareAuthenticator auth.Authenticator, authService auth.Service, authenticationService authentication.Service, blockAdapter block.Adapter, metadataManager auth.MetadataManager, migrator Migrator, collector stats.Collector, cloudMetadataProvider cloud.MetadataProvider, actions actionsHandler, auditChecker AuditChecker, logger logging.Logger, gatewayDomains []string, snippets []params.CodeSnippet, pathProvider upload.PathProvider, usageReporter stats.UsageReporterOperations) http.Handler { | |||
| func Serve(cfg config.Config, catalog *catalog.Catalog, middlewareAuthenticator auth.Authenticator, authService auth.Service, authenticationService authentication.Service, blockAdapter block.Adapter, metadataManager auth.MetadataManager, migrator Migrator, collector stats.Collector, cloudMetadataProvider cloud.MetadataProvider, actions actionsHandler, auditChecker AuditChecker, logger logging.Logger, gatewayDomains []string, snippets []params.CodeSnippet, pathProvider upload.PathProvider, usageReporter stats.UsageReporterOperations) http.Handler { | |||
There was a problem hiding this comment.
Why this change is needed?
There was a problem hiding this comment.
Controller should get the config interface and not an instance of BaseConfig
…nfig-list-8498 # Conflicts: # pkg/api/controller.go
Closes #8498
Change Description
Background
This PR introduces StorageConfigList to our config endpoint response.
The implementation of the StorageConfigList is currently not part of this PR and we return an empty response