Skip to content

Store k-v from config file in 'from_file' dict - #123

Closed
ecebuzz wants to merge 1 commit into
pricingassistant:masterfrom
ecebuzz:master
Closed

Store k-v from config file in 'from_file' dict#123
ecebuzz wants to merge 1 commit into
pricingassistant:masterfrom
ecebuzz:master

Conversation

@ecebuzz

@ecebuzz ecebuzz commented Jul 29, 2016

Copy link
Copy Markdown
Contributor

This prevents key value pairs parsed from config file from leaking into 'default_config'.
Those key value pairs stored in 'from_file' will eventually be merged into 'merged_config'.

And it seems that the merging logic will have the settings from the configuration file overwrite the same settings specified by the environment variable and the arguments.

This prevent key value pairsparsed from config file from leaking into 'default_config'.
Those key value pairs will eventually get merged into 'merged_config'.
@ecebuzz

ecebuzz commented Jul 29, 2016

Copy link
Copy Markdown
Contributor Author

Sorry, my change failed the unit test. Perhaps my understanding is not correct. I found that 'from_file' seems not being used during the configuration merging stage. So I guess that your initial intention might be setting 'from_dict' when parsing the config file.

@sylvinus

sylvinus commented Jul 29, 2016

Copy link
Copy Markdown
Contributor

Hm, thanks again for noticing this!

This area of the code is indeed very confusing. Let me add a few tests!

@sylvinus sylvinus closed this in a8fa962 Jul 29, 2016
@sylvinus

Copy link
Copy Markdown
Contributor

There you go. Thanks again, feel free to open other issues if you see more bugs :)

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.

2 participants