From ca6b29a4e5e0f6510de6780d75e82716f6124d2d Mon Sep 17 00:00:00 2001 From: dttung2905 Date: Wed, 17 Jan 2024 22:17:28 +0000 Subject: [PATCH] Address code review comment Signed-off-by: dttung2905 --- pkg/scalers/scaler.go | 26 ++++++-------------------- pkg/scalers/scaler_test.go | 4 ++-- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/pkg/scalers/scaler.go b/pkg/scalers/scaler.go index 987f17f9655..860c86d2161 100644 --- a/pkg/scalers/scaler.go +++ b/pkg/scalers/scaler.go @@ -171,19 +171,11 @@ func GenerateMetricInMili(metricName string, value float64) external_metrics.Ext type Option func(*configOptions) type configOptions struct { - parameter string useMetadata bool useAuthentication bool useResolvedEnv bool isOptional bool defaultVal interface{} - targetType reflect.Type -} - -func WithParameter(parameter string) Option { - return func(opt *configOptions) { - opt.parameter = parameter - } } func UseMetadata(metadata bool) Option { @@ -216,14 +208,8 @@ func WithDefaultVal(defaultVal interface{}) Option { } } -func WithTargetType(targetType reflect.Type) Option { - return func(opt *configOptions) { - opt.targetType = targetType - } -} - // getParameterFromConfigV2 returns the value of the parameter from the config -func getParameterFromConfigV2(config *scalersconfig.ScalerConfig, options ...Option) (interface{}, error) { +func getParameterFromConfigV2(config *scalersconfig.ScalerConfig, parameter string, targetType reflect.Type, options ...Option) (interface{}, error) { opt := &configOptions{defaultVal: ""} for _, option := range options { option(opt) @@ -234,19 +220,19 @@ func getParameterFromConfigV2(config *scalersconfig.ScalerConfig, options ...Opt var convertedVal interface{} var foundErr error - if val, ok := config.AuthParams[opt.parameter]; ok && val != "" { + if val, ok := config.AuthParams[parameter]; ok && val != "" { foundCount++ if opt.useAuthentication { foundVal = val } } - if val, ok := config.TriggerMetadata[opt.parameter]; ok && val != "" { + if val, ok := config.TriggerMetadata[parameter]; ok && val != "" { foundCount++ if opt.useMetadata { foundVal = val } } - if envFromVal, envFromOk := config.TriggerMetadata[fmt.Sprintf("%sFromEnv", opt.parameter)]; envFromOk { + if envFromVal, envFromOk := config.TriggerMetadata[fmt.Sprintf("%sFromEnv", parameter)]; envFromOk { if val, ok := config.ResolvedEnv[envFromVal]; ok && val != "" { foundCount++ if opt.useResolvedEnv { @@ -255,10 +241,10 @@ func getParameterFromConfigV2(config *scalersconfig.ScalerConfig, options ...Opt } } - convertedVal, foundErr = convertToType(foundVal, opt.targetType) + convertedVal, foundErr = convertToType(foundVal, targetType) switch { case foundCount > 1: - return opt.defaultVal, fmt.Errorf("value for parameter '%s' found in more than one place", opt.parameter) + return opt.defaultVal, fmt.Errorf("value for parameter '%s' found in more than one place", parameter) case foundCount == 1: if foundErr != nil { return opt.defaultVal, foundErr diff --git a/pkg/scalers/scaler_test.go b/pkg/scalers/scaler_test.go index b19b595b972..e4584be5d7a 100644 --- a/pkg/scalers/scaler_test.go +++ b/pkg/scalers/scaler_test.go @@ -256,13 +256,13 @@ func TestGetParameterFromConfigV2(t *testing.T) { for _, testData := range getParameterFromConfigTestDataset { val, err := getParameterFromConfigV2( &scalersconfig.ScalerConfig{TriggerMetadata: testData.metadata, AuthParams: testData.authParams, ResolvedEnv: testData.resolvedEnv}, - WithParameter(testData.parameter), + testData.parameter, + testData.targetType, UseMetadata(testData.useMetadata), UseAuthentication(testData.useAuthentication), UseResolvedEnv(testData.useResolvedEnv), IsOptional(testData.isOptional), WithDefaultVal(testData.defaultVal), - WithTargetType(testData.targetType), ) if testData.isError { assert.NotNilf(t, err, "test %s: expected error but got success, testData - %+v", testData.name, testData)