Skip to content
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

Update configSchema.xml #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sasaraf
Copy link

@sasaraf sasaraf commented Jul 8, 2018

disable the dynamic configuration reload default

disable the dynamic configuration reload default
@ghost
Copy link

ghost commented Jul 11, 2018

@yairogen Could you please take a look? Why is it on by default? what will happen if we'll turn it off.

@yairogen
Copy link
Contributor

@nahsh if you turn it off - you will have to restart the process if you change values in the config file.

@sasaraf
Copy link
Author

sasaraf commented Jul 16, 2018

Hi Yair, :)! ok. we don't mind to keep this feature. however, we would like to extend the PR to the following: to allow us to control (by default as part of our configSchema.xml) what will be our default setting for our services. That is, to load the configuration properties from the resources 'configSchema.xml' in an order that will give our services configSchema.xml priority. We tried override this configuration in our configSchema. but the CompositeConfiguration class does 'getProperty' first on the configuration-lib configSchema and keep returning the default origin value. overriding it only as part of config.properties is error prune on our behalf.

public Object getProperty(String key) { Configuration firstMatchingConfiguration = null; for (Configuration config : configList) { if (config.containsKey(key)) { firstMatchingConfiguration = config; break; } }

The firstmatching is in the origin order of the configList instead of opposite. if i open such PR will you approve?

@KerenSi
Copy link

KerenSi commented Jul 16, 2018

@yairogen tagging Yair :)

@sasaraf
Copy link
Author

sasaraf commented Jul 16, 2018

Thanks @KerenSi ;)

@yairogen
Copy link
Contributor

yairogen commented Jul 16, 2018 via email

@sasaraf
Copy link
Author

sasaraf commented Jul 16, 2018

ok. i see now it is internal getProperty of apache ConfigurationConfiguration extension .so it will require managing the configList in reversed order. will take me more than couple of minutes to do the actual change. hopefully i will get to it by the end of this week b'h.

@yairogen
Copy link
Contributor

yairogen commented Jul 17, 2018 via email

@sasaraf
Copy link
Author

sasaraf commented Jul 17, 2018 via email

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