-
Notifications
You must be signed in to change notification settings - Fork 22
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
Provide a mechanism to detect ConfigValue changes #63
Comments
This is a useful addition in general. Concise API, easy to implement. However, the downside as I understand it is that it will only catch changes made from the Config API, but not external changes to ConfigSources, say etcd or a database. So it must be clearly to users that external changes might not be catched. While not 100% removing the problem, a companying addition might be a mean for a ConfigSource to emit change notifications when the underlying store supports such detection/notification. The problem there is how to propagate such notification, since it might not be part of the effective configuration if a source with higher precedence masks the property - in this case the it would be counter intuitive for the application to receive a change notification, since the effective application hasn't changed. |
If this is a throwaway example don't worry, but otherwise |
Hi @rgielen! Thanks for your feedback. Yes, it's not intended to get actively notified asynchronously if anything deep inside the Config changes. |
So do I understand correctly that to trigger an event when a value is changed (or soon after), it's necessary to poll config for that value? Something like this:
If yes, then it's cool because it's clear that the listeners are executed in the current thread and no other thread needs to be created. However, I would consider adding special methods for this to make it more obvious. For example: |
Yes, it's fully synchronous on the same thread. That has benefits and of course the downside that you really only get the callback if the getValue() or getOptionalValue() get invoked. It's basically internally the following code
So it's not a full replacement for the Config#registerOnConfigChange we had in earlier versions. This would have been fully 'pro-active', but also would have added quite a few new problems, like getting notified asynchronously on a new thread without any EE set up, etc... Oh and thanks for the feedback! |
Oh and I got asked for other use cases. The things I've seen is
|
will work on it due to popular demand. |
Great, any news on the EDR? |
Splitting this out from #9 into a separate ticket as discussed in our last EG meeting.
When people make use of ConfigValue they often want to trigger some action when a new value get's picked up.
Remember that ConfigValue is only needed for configured values which might change during runtime.
Often a user wants to be aware when the underlying value got changed. Either to log it out, or to perform some other code.
This is essentially done by registering a lambda as callback which gets called whenver the underlying config gets accessed an a change gets detected.
https://github.com/struberg/ConfigJSR-1/blob/i9_configValue/api/src/main/java/javax/config/ConfigValue.java#L162
Here is a code examples to highlight why this is an important use case:
This is really important for our Ops team. Otherwise they don't really have a clue when the system eventually picked up new values!
The onChange callback might also be used to e.g. clean cached values, etc. It's really very versatile and an important use case!
The text was updated successfully, but these errors were encountered: