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

Defaults/Overrides auto-loading - necessary? #18

Open
bitprophet opened this issue Sep 29, 2014 · 2 comments
Open

Defaults/Overrides auto-loading - necessary? #18

bitprophet opened this issue Sep 29, 2014 · 2 comments

Comments

@bitprophet
Copy link
Contributor

I'm trying to use etcaetera in a "case sensitive" manner where the incoming config sources (files etc) are not forced into upper- or lowercase but simply retain the literal values.

(There's no hard requirement for this on my end [yet], but it feels like the sane default to me. If a user decided they wanted a SillyCapsName key in their config file, my tool automatically changing it to SILLYCAPSNAME or sillycapsname feels like a nasty surprise :()

I tried to do Config(formatter=noop) (where: def noop(s): return s) to implement this, and found that the Defaults and Overrides adapters gum it up by calling self.load() in their __init__ - so their default uppercase formatters execute before the central one defined on Config gets a chance to (by which time the "real" key data has been lost).

End result is that I presently need to override formatter= in multiple places instead of just Config. Not a deal killer, but inelegant and not DRY :)

This brings me to my actual questions:

  • Why do these classes auto-load? I don't see the use case off hand, but it's apparently intentional as the tests fail with those lines removed.
  • What's the rationale behind the default to uppercase? It's not really mentioned in the documentation (besides the Python-module angle which is understandable).
  • Would you consider either change I alluded to above - changing the default adapter to a noop (preferable), or having Defaults and Overrides cease auto-loading? I'm willing to make a PR if necessary.
@oleiade
Copy link
Owner

oleiade commented Oct 1, 2014

Have to read the code again to answer precisely to your questions and find a proper solution to the problem you're pointing, which, by the way sounds completly relevant to me.

However about the questions you asked:

  • Defaults and Override where added to process in order to be able to create an initial state for your configuration. It fits with most of my projects use cases: when for example you need to creates Defaults settings for a given set of variables, and update them only if an adapter is overriding their value.
    Override should always be called last, for if you want to ensure that whatever adapter values are inserted in your configuration, you will force their value. It can be usefull to insert constant values in your settings, or just debugging without touching your adapters input.
    Anyway, I remember that I had some frustrations while using this system as is, and I'm open to a refactor. Let me know if it answers to your question :-)
  • As a default etcaetera used uppercase to simplify the Config object keys deduplication. I wanted to make sure that every keys where unique. To somehow protect the user from runtime bugs, with duplicate keys overriding each other implicitly. However I realize that maybe this uppercased thing was not the proper solution and introduce the ability to write your own transformers. I agree this can be improved and I am open to modifications there as long as they protect the explicit is better implicit principle and don't let internal implicit bugs that would let the user helpless.
  • I think adding a noop transformer of yours in the set of current helpers is a good idea!

Let me know what you think !

@bitprophet
Copy link
Contributor Author

  • Re: Defaults and Override, I get why they exist and I'm planning to use both in my setup :) My question was: why are they different from the other adapters like File, which do not call self.load() in __init__?
    • I.e. the pattern for the other adapter types is: instantiate adapters; register with Config; call Config.load; Config.load then calls each adapter's .load() in turn (handing in the Config object's formatter value).
    • This works great; it honors the Config object's formatter (allowing a single place to configure the formatter); no fuss, no muss.
    • However, Defaults and Override call self.load() in their own __init__ which means that they load (and mutate! via their default formatter) their config data immediately, instead of deferring to whenever the Config object calls their load.
    • And I just don't get why they're done that way :) Why not have them behave like File, Env etc?
  • Re: uppercase & deduping, my opinions as a longtime Python developer:
    • Users will often be confused when their inputs change unexpectedly; similarly, most users would not expect this sort of "easily make some YAML/JSON/Python config files available to me as a Python dict" tool to mutate the data by default.
    • I haven't looked at the code in depth but I presume it's possible to warn about (or abort on) potentially accidental duplicate keys (e.g. foobar vs FooBar appearing at the same level) without actually mutating the end result.
    • Another argument is that Python strings, as well as YAML and JSON keys, are all natively case-sensitive, so by formatting you can actually introduce collision/lossy behavior where there would not otherwise have been any. If a user for some reason intended to allow both foobar and FooBar keys, there's now ambiguity as to which one would/should "win" when they both become the single key FOOBAR.
    • My overall 2 cents is, if you feel strongly that protection against accidental key duplication is important, a more user-friendly approach might be to make it an opt-in feature & then add a very visible note to the documentation saying "hey! this is a thing that can happen! we recommend turning this safety feature on". But having it on by default is (IMHO) likely to cause more problems than it catches.
    • All that said - as long as I have an easy way to change the default behavior in one spot (thus the 1st top level bullet here :)) that's all I primarily care about. Just offering an opinion as someone who's dealt with many types of Python programmer over the years ;)

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

No branches or pull requests

2 participants