Skip to content

Commit

Permalink
Address code review comment
Browse files Browse the repository at this point in the history
Signed-off-by: dttung2905 <[email protected]>
  • Loading branch information
dttung2905 committed Jan 17, 2024
1 parent ce5b4ca commit ca6b29a
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 22 deletions.
26 changes: 6 additions & 20 deletions pkg/scalers/scaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/scalers/scaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit ca6b29a

Please sign in to comment.