-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Refactor reload (2) #1790
Refactor reload (2) #1790
Conversation
/** | ||
* used for the event based reloading. | ||
*/ | ||
public static boolean reload(String target, String sourceAsString, PropertySourceLocator locator, |
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.
I want to achieve a very simple thing here : only two single "entry points" into this class. Only two public methods that both polling and event based reload should call, nothing else should be public. And they do exactly the same thing
List<? extends MapPropertySource> existingSources = findPropertySources(existingSourcesType, environment); | ||
|
||
if (existingSources.isEmpty()) { |
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.
I am adding this code:
if (existingSources.isEmpty()) {
LOG.debug(() -> "no existingSources found, reload will not happen");
return false;
}
This was actually already part of the polling code, but not from the event based one. So this aligns them.
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 PR can only be created once you are OK with this one and it ends up in main
...
*/ | ||
@Deprecated(forRemoval = false) |
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.
since this is public already, I can't make it package private, but I will create a PR that will make it such for the next major release
if (!currentConfigMapSources.isEmpty()) { | ||
changedConfigMap = changed(locateMapPropertySources(this.propertySourceLocator, this.environment), | ||
currentConfigMapSources); | ||
boolean changedConfigMap = ConfigReloadUtil.reload(propertySourceLocator, environment, propertySourceClass); |
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.
instead of calling : findPropertySources
, then locateMapPropertySources
, we now call reload
, that internally does the same thing
@ryanjbaxter this one is ready to be looked at too. (I have two PRs open) Thank you! |
No description provided.