Skip to content
This repository has been archived by the owner on Sep 12, 2018. It is now read-only.

Config rehaul #444

Merged
merged 10 commits into from
Jun 30, 2014
Merged

Config rehaul #444

merged 10 commits into from
Jun 30, 2014

Conversation

dmp42
Copy link
Contributor

@dmp42 dmp42 commented Jun 23, 2014

This is mainly to fix our various problems with configuration typage/env that grew sore in the aftermath of the standalone True debacle:

Incidentally, this also lay code to fix #440 and does fix #442.

BEWARE that the Configuration object behavior changed and that this may have unexpected and tricky impact, so, please review (ping my favorites-eagle-eyed reviewers @shin- and @wking) and if possible test.

Please note I'm not changing for the sake of it - I'm changing so that we have something that behaves rationally - and that we know exactly how.

In a nutshell:

  • environment derived values are now typed properly (eg: they are YAML parsed) - which means MY_ENV_VAR=True will give True and MY_ENV_VAR='["foo", "bar"]' will give a python list, etc
  • default values (for env derived values) are now typed properly (same as above)
  • the config object is now usable with dot notation, as deep as there is values. eg: cfg.foo.bar.baz
  • the config object is no longer usable with the get method
  • dict values are themselves instances of the Config class
  • a non existent key with return None
  • falsy values are no longer coalesced
  • the common flavor is no longer forcefully inherited inside python code, but the heritage is declared using YAML - this is DEFINITELY a point where you need to update configuration files and that will break for people not paying attention
  • as long as a key exists in YAML, it WILL be returned (eventually None) - this is an important change from the previous behavior where falsy values would mean the key is not returned at all (propagating to the parent dict which in turn might have returned None previously if no key were Trusy). This is especially true for the mirroring and cache config (code has been adjusted)
  • we now have extensive tests to characterize all this
  • default values were previously scattered (half in code, half in config.yaml). This is no longer the case, and all defaults are now specified in the config file. This WILL break for people not paying attention, specially the redis code which no longer makes "default" assumptions

@shin-: do we need config/config_mirroring.yml anymore? or can I remove it?

Mangled Deutz added 8 commits June 23, 2014 12:57
Docker-DCO-1.1-Signed-off-by: Mangled Deutz <olivier@webitup.fr> (github: dmp42)
Docker-DCO-1.1-Signed-off-by: Mangled Deutz <olivier@webitup.fr> (github: dmp42)
Docker-DCO-1.1-Signed-off-by: Mangled Deutz <olivier@webitup.fr> (github: dmp42)
- replace get(key) and ['key'] calls by dotted notation
- removed defaults from the codebase (as they now all live in the config)
- tweak existence tests
- simplified calls to load() when possible
- assume proper typage

Docker-DCO-1.1-Signed-off-by: Mangled Deutz <olivier@webitup.fr> (github: dmp42)
+ enhanced typage tests

Docker-DCO-1.1-Signed-off-by: Mangled Deutz <olivier@webitup.fr> (github: dmp42)
Docker-DCO-1.1-Signed-off-by: Mangled Deutz <olivier@webitup.fr> (github: dmp42)
Docker-DCO-1.1-Signed-off-by: Mangled Deutz <olivier@webitup.fr> (github: dmp42)
Docker-DCO-1.1-Signed-off-by: Mangled Deutz <olivier@webitup.fr> (github: dmp42)
@dmp42 dmp42 added this to the 0.8 milestone Jun 23, 2014
@petarmaric
Copy link

Thanks for keeping me happy 👍

Nice work, this pull request LGTM.

Docker-DCO-1.1-Signed-off-by: Mangled Deutz <olivier@webitup.fr> (github: dmp42)
@mdub
Copy link

mdub commented Jun 24, 2014

Nice! 👍

Docker-DCO-1.1-Signed-off-by: Olivier Gambier <olivier@docker.com> (github: dmp42)
@dmp42
Copy link
Contributor Author

dmp42 commented Jun 25, 2014

@shin- ping

@shin-
Copy link
Contributor

shin- commented Jun 26, 2014

LGTM

@dmp42
Copy link
Contributor Author

dmp42 commented Jun 27, 2014

Thanks @shin- !
@wking would you have time to exert your x-rays on this before it gets in and mistakenly hurt unicorns?

dmp42 added a commit that referenced this pull request Jun 30, 2014
@dmp42 dmp42 merged commit d695902 into master Jun 30, 2014
@dmp42 dmp42 deleted the config-enhance branch June 30, 2014 14:44
@shin- shin- mentioned this pull request Jun 30, 2014
gierschv added a commit to ovh/docker-registry-driver-swift that referenced this pull request Aug 14, 2014
The docker-registry-core >= 2.0.0 (docker-registry >= 0.8) breaks the
compatibility with the config usage (docker-archive/docker-registry#444)

This commit fixes bacongobbler#12

Signed-off-by: Vincent Giersch <vincent.giersch@ovh.net>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't run as root
4 participants