Skip to content

Conversation

@venilnoronha
Copy link
Contributor

Hi,

I've moved RefreshEndpoint's logic to RefreshSupport and delegated the refresh call to it. One can now use RefreshSupport outside actuator. Please verify and merge.

My ICLA Number: 149720151120072904

Thanks,
Venil Noronha

*
* @author Venil Noronha
*/
public class RefreshSupport {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not so keen on the name. Maybe EnvironmentRefresher? @dsyer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, need a better name. We already have EnvironmentManager. What about ContextRefresher? I like the way that ApplicationEvents provide an extensibility already, though (you can listen for the event if you want to refresh more stuff). Maybe the ContextRefresher could just emit an event, and all the other features can be listeners?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And log the keys as a different type of event?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've renamed the class to ContextRefresher. I didn't get the ApplicationEvents part. Are we talking about publishing a new event along with EnvironmentChangeEvent?

2. Moved ContextSupport bean creation to RefreshAutoConfiguration.
@venilnoronha
Copy link
Contributor Author

@spencergibb I've done the necessary changes. Could you please review? Also, do guide me with the ApplicationEvents change if need be.

@spencergibb
Copy link
Member

@dsyer is this ok after we've gone into RC mode?

@dsyer
Copy link
Contributor

dsyer commented Mar 31, 2016

Fine I think (it's not really a new feature).

spencergibb added a commit that referenced this pull request Apr 1, 2016
* pull103:
  Moved RefreshEndpoint logic to ContextRefresher.
@spencergibb
Copy link
Member

Squashed and merged via 1e71c56

@spencergibb spencergibb closed this Apr 1, 2016
@venilnoronha
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants