1996fanrui commented on PR #23994:
URL: https://github.com/apache/flink/pull/23994#issuecomment-1869905090
Thanks @RocMarshal for the quick review!
> if we run the followed case, will be our expected results consistent with
the current fixed results?
>
> ```
> @Test
> void testUnknownCases() {
> Configuration original = new Configuration();
> final DelegatingConfiguration delegatingConf =
> new DelegatingConfiguration(original, "prefix.");
>
> // Test for integer
> ConfigOption<Integer> integerOption =
>
ConfigOptions.key("integer.key").intType().noDefaultValue();
>
> // integerOption doesn't exist in delegatingConf, and it should be
overrideDefault.
> original.setInteger(integerOption, 1);
> assertThat(delegatingConf.getInteger(integerOption,
2)).isEqualTo(2);
>
> // integerOption exists in delegatingConf, and it should be value
that set before.
> delegatingConf.setInteger(integerOption, 3);
> assertThat(delegatingConf.getInteger(integerOption,
2)).isEqualTo(3);
>
> delegatingConf.removeConfig(integerOption);
> System.out.println(delegatingConf.get(integerOption));
>
> }
> ```
>
> If not, it may be a bug ?
It's indeed a bug. The `removeConfig` and `removeKey` of
`DelegatingConfiguration` missed the `prefix` as well. I didn't notice them in
the beginning. Would you like to fix it? If yes, feel free to take it, and I'm
glad to help review.
Also, I have addressed your other comments. Please help double-check in your
free time, thanks~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
