fix bug: some OpAMPBridge values are always overriden by default #3934
Conversation
|
@jaronoff97 Can you review this plz? |
|
Instead of trying to detect if a value was loaded from file, I'd prefer if we took an approach similar to #3962. That is:
WDYT @jaronoff97 ? |
|
@jaronoff97 @swiatekm I fixed. thank you |
| @@ -212,10 +273,9 @@ func TestLoad(t *testing.T) { | |||
| } | |||
| got := NewConfig(logr.Discard()) | |||
| err := LoadFromFile(got, tt.args.file) | |||
| if !tt.wantErr(t, err, fmt.Sprintf("Load(%v)", tt.args.file)) { | |||
| if err != nil && tt.wantErr(t, err, fmt.Sprintf("Load(%v)", tt.args.file)) { | |||
There was a problem hiding this comment.
@jaronoff97
this line didn't work before.
I understood that if err is not nil and wantErr is satisfied, the process should not continue any further.
That's why I changed it this way. Isn't that correct?
There was a problem hiding this comment.
how did it not work before? We want to check both cases here, i.e. if we expect an error (where wantErr = assert.Error) and there is no error, then we should fail. And if we don't expect an error and get one we should fail too.
There was a problem hiding this comment.
@jaronoff97
I understood that in cases where wantErr is not NoError, meaning when an error should occur, it was intended to terminate at this return statement and not execute the following code. However, in reality, even in cases where an error should occur, it wasn't returning from the if statement and was continuing to execute the next lines.
It seems to be fixed now.
Description:
I fixed a bug where certain variables that could be set via both the config file and CLI were always being overridden by their default values, even if a config file was present. Now, when a config file exists, its values take precedence over the defaults, and if both a config file and CLI input are provided, the CLI input takes precedence.
@jaronoff97
Link to tracking Issue(s):
Testing:
Documentation: