search

bw2 / ConfigArgParse

A drop-in replacement for argparse that allows options to also be set via config files and/or environment variables.
MIT License
723 stars 121 forks source link

no way to have mixed-case auto_env_var_prefix makes it hard to work with bamboo #182

Open systematicguy opened 4 years ago

systematicguy commented 4 years ago

First of all, I am overall very pleased with configargparse module as it saves me the legwork I did for at least 5 of my previous projects - now I have configargparse at my fingertips with an acceptable amount of compromises to make. Thanks for your work with this!

To the topic: We use Atlassian Bamboo for ci/cd, and unfortunately Bamboo plan variables which are injected as environment variables, are prefixed with "bamboo_" when running on unix agents.

Now I found in https://github.com/bw2/ConfigArgParse/blob/1.0/configargparse.py#L426 that we do the upper for the whole concatenated thing, and thus e.g. bamboo_TOOL_PARAM will not be picked up, only BAMBOO_TOOL_PARAM. I also see this mentioned in the docstring. I see the idea of upper-casing the param names basically a good thing (after all e.g. on windows there are only upper-case envvars), but sometimes we don't have a way to force the "user" (in this case the ci system) to do this or that. On the long term I would like to see the prefix allowing it to be mixed-case, instead of doing the os.environ patching (which I apply now as a horrible workaround before parsing).

I am about to make my first PR in my life about this as the fix is really easy and non-intrusive, but wanted to ask

  1. if there is any hard opinion not to allow the prefix to be mixed-case
  2. which version you prefer: a: checking for both (pseudo: "{}{}".format(prefix, key.upper(),"{}{}".format(prefix.upper(), key.upper()) b: checking only ("{}{}".format(prefix, key.upper())
bw2 commented 4 years ago

Hi @kottalovag , glad you're planning to submit a PR. I would go with 2a. in case somebody already has code that depends on the current behavior.

systematicguy commented 4 years ago

Hi @bw2 thanks for the positive feedback. Just a heads-up, I have started working on this, and then slept over it to have a refined solution. I have basically implemented the change, I will now have to look into the tests (and of course cover the new cases). https://github.com/kottalovag/ConfigArgParse/commits/feature/flexible-auto_env_var_prefix

c01o commented 3 years ago

Same problem here, it's nice if we have a flag or something to stop upcasing.