loganasherjones / yapconf

Yet Another Python Configuration
http://yapconf.readthedocs.io/en/stable/
MIT License
18 stars 4 forks source link

Env name #12

Closed hazmat345 closed 6 years ago

hazmat345 commented 6 years ago

This is in regards to #11.

The first commit (8e1420d) adds an env_name field to explicitly call out that you must have knowledge of the spec-level prefix that will be applied as well as how nested prefixes work in order to correctly name your environment variable.

The second commit (594364f) tweaks this behavior by always applying the prefixes to whatever the 'base' env_name would be and updates the test to reflect that.

I definitely do see the value in being able to 'override' the prefix mechanism, especially as it relates to nested items. This change would mean that if you have a spec with prefix MY_APP_ and this definition:

'database': {
  'type': 'dict',
  'items': {
    'host': {
      'type': 'str',
      'env_name: 'HOST'
    }
  }
}

The environment variable to set will be MY_APP_DATABASE_HOST. I think this is better in that you don't need MY_APP_ as part of the spec definition, but worse in that there is no way to remove the DATABASE part of the name.

I think I can see value in all of these scenarios - probably the 'correct' thing to do is a flag that would control which prefixes get applied, something like apply_prefixes with possible values all, app_only, nesting_only, and none.

This PR is a quick proof of concept that I don't think you should accept yet, but I didn't want to spend any real time on this without your OK.

Let me know your thoughts 😄

loganasherjones commented 6 years ago

As this was to address #11 I believe #18 fixed this issue. Feel free to re-open if you disagree.