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

Move service.ConfigMapConverterFunc to config.MapConverterFunc #4673

Merged
merged 1 commit into from
Jan 13, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- Change Properties Provider to be a Converter (#4666)
- Define a type `WatcherFunc` for onChange func instead of func pointer (#4656)
- Remove deprecated `configtest.LoadConfig` and `configtest.LoadConfigAndValidate` (#4659)
- Move service.ConfigMapConverterFunc to config.MapConverterFunc (#4673)

## 💡 Enhancements 💡

Expand Down
5 changes: 5 additions & 0 deletions config/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ const (
KeyDelimiter = "::"
)

// MapConverterFunc is a converter function for the config.Map that allows distributions
// (in the future components as well) to build backwards compatible config converters.
// TODO: Consider to add a context as the first argument.
type MapConverterFunc func(*Map) error
Comment on lines +35 to +36
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that this is a valuable addition and should be done now.


// NewMap creates a new empty config.Map instance.
func NewMap() *Map {
return &Map{k: koanf.New(KeyDelimiter)}
Expand Down
3 changes: 1 addition & 2 deletions config/configmapprovider/properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ import (
// Properties must follow the Java properties format, key-value list separated by equal sign with a "."
// as key delimiter.
// ["processors.batch.timeout=2s", "processors.batch/foo.timeout=3s"]
// TODO: Find the right place for service.ConfigMapConverterFunc and return that. Currently this will cause circular dependency.
func NewOverwritePropertiesConverter(properties []string) func(*config.Map) error {
func NewOverwritePropertiesConverter(properties []string) config.MapConverterFunc {
return func(cfgMap *config.Map) error {
return convert(properties, cfgMap)
}
Expand Down
10 changes: 3 additions & 7 deletions service/config_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ type ConfigProvider interface {
type configProvider struct {
locations []string
configMapProviders map[string]configmapprovider.Provider
cfgMapConverters []ConfigMapConverterFunc
cfgMapConverters []config.MapConverterFunc
configUnmarshaler configunmarshaler.ConfigUnmarshaler

sync.Mutex
Expand All @@ -86,7 +86,7 @@ type configProvider struct {
func NewConfigProvider(
locations []string,
configMapProviders map[string]configmapprovider.Provider,
cfgMapConverters []ConfigMapConverterFunc,
cfgMapConverters []config.MapConverterFunc,
configUnmarshaler configunmarshaler.ConfigUnmarshaler) ConfigProvider {
return &configProvider{
locations: locations,
Expand All @@ -97,10 +97,6 @@ func NewConfigProvider(
}
}

// ConfigMapConverterFunc is a converter function for the config.Map that allows distributions
// (in the future components as well) to build backwards compatible config converters.
type ConfigMapConverterFunc func(*config.Map) error

// NewDefaultConfigProvider returns the default ConfigProvider, and it creates configuration from a file
// defined by the given configFile and overwrites fields using properties.
func NewDefaultConfigProvider(configFileName string, properties []string) ConfigProvider {
Expand All @@ -110,7 +106,7 @@ func NewDefaultConfigProvider(configFileName string, properties []string) Config
"file": configmapprovider.NewFile(),
"env": configmapprovider.NewEnv(),
},
[]ConfigMapConverterFunc{
[]config.MapConverterFunc{
configmapprovider.NewOverwritePropertiesConverter(properties),
configprovider.NewExpandConverter(),
},
Expand Down
4 changes: 2 additions & 2 deletions service/config_provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func TestConfigProvider_Errors(t *testing.T) {
name string
locations []string
parserProvider map[string]configmapprovider.Provider
cfgMapConverters []ConfigMapConverterFunc
cfgMapConverters []config.MapConverterFunc
configUnmarshaler configunmarshaler.ConfigUnmarshaler
expectNewErr bool
expectWatchErr bool
Expand Down Expand Up @@ -142,7 +142,7 @@ func TestConfigProvider_Errors(t *testing.T) {
"mock": &mockProvider{},
"file": configmapprovider.NewFile(),
},
cfgMapConverters: []ConfigMapConverterFunc{func(c *config.Map) error { return errors.New("converter_err") }},
cfgMapConverters: []config.MapConverterFunc{func(c *config.Map) error { return errors.New("converter_err") }},
configUnmarshaler: configunmarshaler.NewDefault(),
expectNewErr: true,
},
Expand Down
4 changes: 1 addition & 3 deletions service/internal/configprovider/expand.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ import (
)

// NewExpandConverter returns a service.ConfigMapConverterFunc, that expands all environment variables for a given config.Map.
//
// This does not directly return service.ConfigMapConverterFunc to avoid circular dependencies, not a problem since it is internal.
func NewExpandConverter() func(*config.Map) error {
func NewExpandConverter() config.MapConverterFunc {
return func(cfgMap *config.Map) error {
for _, k := range cfgMap.AllKeys() {
cfgMap.Set(k, expandStringValues(cfgMap.Get(k)))
Expand Down