alievmirza commented on a change in pull request #114:
URL: https://github.com/apache/ignite-3/pull/114#discussion_r627608119
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/storage/ConfigurationStorage.java
##########
@@ -44,6 +44,8 @@
* Add listener to the storage that notifies of data changes.
* @param listener Listener.
*/
+ // TODO: seems that it's not needed to have an ability to set several
listeners to storage, as far as only one is responsible
Review comment:
This is not obvious to implement as far as
`DistributedConfigurationStorage` instance is created before
ConfigurationChanger, but we need to set
`org.apache.ignite.configuration.ConfigurationChanger#updateFromListener` to
`DistributedConfigurationStorage` at some point, so the constructor doesn't fit
here. I would do cush changes in a different ticket
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/storage/ConfigurationStorage.java
##########
@@ -44,6 +44,8 @@
* Add listener to the storage that notifies of data changes.
* @param listener Listener.
*/
+ // TODO: seems that it's not needed to have an ability to set several
listeners to storage, as far as only one is responsible
Review comment:
This is not obvious to implement as far as
`DistributedConfigurationStorage` instance is created before
`ConfigurationChanger`, but we need to set
`org.apache.ignite.configuration.ConfigurationChanger#updateFromListener` to
`DistributedConfigurationStorage` at some point, so the constructor doesn't fit
here. I would do cush changes in a different ticket
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
