fix (kubernetes-client-api) : Config's autoConfigure should disable auto configuration
#6153
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
3a2cfc5 to
c97e1e4
Compare
18 tasks
manusa
reviewed
Jul 19, 2024
c1a47c8 to
dbbe657
Compare
manusa
reviewed
Jul 22, 2024
11 tasks
7152199 to
7a3bd1d
Compare
manusa
reviewed
Aug 6, 2024
| OAuthTokenProvider oauthTokenProvider, Map<String, String> customHeaders, int requestRetryBackoffLimit, | ||
| int requestRetryBackoffInterval, int uploadRequestTimeout, boolean onlyHttpWatches, NamedContext currentContext, | ||
| List<NamedContext> contexts, boolean autoConfigure) { | ||
| autoConfigure(this, null); |
There was a problem hiding this comment.
This looks weird. Why is autoconfigure always invoked? It wasn't before (in some of the methods)
manusa
reviewed
Aug 6, 2024
Comment on lines
395
to
396
| this.apiVersion = Objects.equals(this.apiVersion, apiVersion) ? "v1" : apiVersion; | ||
| this.namespace = Objects.equals(this.namespace, namespace) ? null : namespace; |
There was a problem hiding this comment.
these also look weird?
why is the variable overridden to an arbitrary (magic) value in case the current and provided values match?
952b090 to
d20f386
Compare
b8eca9b to
b2e2a09
Compare
…le auto configuration
`Config`'s autoConfigure field should behave as per expectations. We
need to make changes to ConfigBuilder and ConfigFluent in order to
achieve this.
- Add a new Constructor with additional boolean arguments `autoConfigure` and
`shouldSetDefaultValues`
- Add a new class `SundrioConfig` that would extend `Config` for
generating the Sundrio Builder for Config
- Move generated classes `ConfigBuilder` and `ConfigFluent` to `src/main/java`
- `ConfigBuilder` would extend generated SundrioConfigBuilder and
override the `build()` method by calling `disableAutoConfig()` if
`autoConfigure` is nto provided by the user. Earlier `ConfigBuidler` was calling `new Config()`
(this was enabling auto configuration)
- Make all fields in Config class use boxed types instead of primitive
types so that we can distinguish whether they have been configured by
user.
Signed-off-by: Rohan Kumar <rohaan@redhat.com>
b2e2a09 to
49cb4cb
Compare
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fix #6137
Config's autoConfigure field should behave as per expectations.We need to make changes to ConfigBuilder and ConfigFluent in order to achieve this.
autoConfigureandshouldSetDefaultValuesSundrioConfigthat would extendConfigfor generating the Sundrio Builder for ConfigConfigBuilderandConfigFluenttosrc/main/javaConfigBuilderwould extend generated SundrioConfigBuilder and override thebuild()method by callingdisableAutoConfig()ifautoConfigureis not provided by the user. EarlierConfigBuidlerwas callingnew Config()(this was enabling auto configuration)Type of change
test, version modification, documentation, etc.)
Checklist