-
Notifications
You must be signed in to change notification settings - Fork 1k
Unify config properties #2047
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unify config properties #2047
Conversation
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
| @DefaultValue("false") boolean enabled, String name, String namespace, boolean useNameAsPrefix, | ||
| @DefaultValue("true") boolean includeProfileSpecificSources, boolean failFast, | ||
| @DefaultValue RetryProperties retry, @DefaultValue("BATCH") ReadType readType) { | ||
| public final class SecretsConfigProperties extends SourceConfigProperties { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this entire PR is based on this (and same configmap) change.
In the secrets and configmaps normalization and determining the sources, we do the same exact code. Meaning methods SecretsConfigProperties::determineSources and ConfigMapConfigProperties::determineSources, do the same exact thing. The only difference is where the result is stored.
As such, let's create a SourceConfigProperties that does all that work, and these two classes simply extend it.
Nothing changes besides this, same code, same logic.
The many changes in this PR are triggered by the fact that I changed a couple or arguments with places, so that ConfigMapConfigProperties and SecretsConfigProperties match exactly.
|
@ryanjbaxter one more in main to take a look. Ive left a comment explaining it. Thank you |
No description provided.