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

feat(config): support MERGE-PRIORITIES for Config items #448

Closed
wants to merge 2 commits into from

Conversation

ankostis
Copy link
Contributor

(I hope you don't get too mad at me for trying...)

Retrofitted loader.Config class to support merging-priorities for specific values based on ranks (e.g. integers). Now update_config() always respects priorities. The purpose it to use it to fully support also #99 environment-variable priorities.

For this PR, the new features are used also to deprecate the need to re-update Application.cli_config on top of file-configs.

Implementation

  • A lot of effort was put to have code and the memory-footprint as small as possible.
    • Config instances by default don't waste extra memory - default priority resolved from _merge_priority class attribute.
    • Possible to set a single rank(number) that propagates to all config-object values; these apply to 1st level only. But it can be set recursively to inner config-objects (wasting an in per config-object).
    • To support different ranks in the same config-object, the _merge_priority is upgraded to dict.
    • The original idea in Envvars #439 of storing tags (a new dict) into config ibjects would be an overkill.
    • Have to study performance-tax on the merges.
  • A new Config.substract() method is used to limit value-changes, taking into account priority-ranks.
    This is useful to preserve traits that have been directly modified between loading of configs from different sources.
  • The ranks must be comparable. besides that, the code does not enforce any other limitation on their type APART from setting the default Config._merge_priority = 0 which in effect "force" the use of integers.

@minrk
Copy link
Member

minrk commented Aug 20, 2017

I appreciate the effort, but as I described in #439, I think we should not introduce the concept of priority to config objects.

@minrk minrk added this to the no action milestone Aug 20, 2017
@minrk minrk closed this Aug 20, 2017
ankostis added a commit to ankostis/traitlets that referenced this pull request Aug 20, 2017
@ankostis
Copy link
Contributor Author

ankostis commented Jan 12, 2018

For whoever lands here, latest changes for the same functionality in ankostis@confranks.

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