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

Environment variable override #787

Closed
rpfernandezjr opened this issue Nov 8, 2019 · 11 comments · Fixed by #1425
Closed

Environment variable override #787

rpfernandezjr opened this issue Nov 8, 2019 · 11 comments · Fixed by #1425
Assignees
Labels
📈 enhancement New feature or request

Comments

@rpfernandezjr
Copy link

rpfernandezjr commented Nov 8, 2019

Describe your problem or feature you'd like added

In order to deploy into a kubernetes infrastructure with more ease, I'd like the ability for any variables defined within the environment, to supersede any entries defined in the env.json file that is provided.

Describe the solution you'd like

I'd like for env_sample.json to be loaded as the default configuration. Any value defined in the environment during initialization, should override what has been loaded as default.

For example, if env_sample.json was loaded as our default configuration, it contains the following entries for log levels

DJANGO_LOG_LEVEL: DEBUG
DJANGO_DEBUG: true
DJANGO_TEMPLATE_DEBUG: true
ROOT_LOG_LEVEL: INFO

If my environment were to have the following defined

export DJANGO_LOG_LEVEL=WARN
export ROOT_LOG_LEVEL=DEBUG

I'd like the variables from my environment to override the default settings that were loaded from the json file. In the end we end up with these values being loaded.

DJANGO_LOG_LEVEL: WARN
DJANGO_DEBUG: true
DJANGO_TEMPLATE_DEBUG: true
ROOT_LOG_LEVEL: DEBUG

Additional context

If it heps any, I was accomplishing this with this snippet in settings.py

if os.getenv("DJANGO_CONFIG_DEFAULTS"):
    # optionally load settings from an environment variable
    with open(os.getenv("DJANGO_CONFIG_DEFAULTS")) as f:
        CONFIG = json.load(f)

    # override defaults with variables defined by the environment
    ENV = {**CONFIG, **os.environ}
else:
    ENV = os.environ

downside of my snippent is that everything from the environment was loaded as a string. I then needed to cast variables that I knew were a bool (str_2_bool) or int when assigning from ENV

@rpfernandezjr rpfernandezjr added the 📈 enhancement New feature or request label Nov 8, 2019
@emilychee
Copy link

Hi! My partner @apalmer16 and I are currently students enrolled in EECS 481. We recently saw a presentation regarding MyLA in class and wanted to contribute to fixing an open source issue. We just wanted to let you know that we will be attempting to implement this feature!

@justin0022
Copy link
Collaborator

Thanks @emilychee, we appreciate your help!

@jonespm
Copy link
Member

jonespm commented Nov 19, 2019

In the code we should have defaults defined that match the json but inevitably that does get out of sync with an external configuration file.

For instance the settings.py has
DEBUG = ENV.get('DJANGO_DEBUG', True)

I'd personally like to switch to defining most of the settings in Django admin using a live settings package. I'm not sure which one of those to pick. Probably one that's been updated recently/often and is going to be compatible with Django 2. Then we could load all of the defaults into the database via migrations.

@apalmer16
Copy link

So if we decided to test using django-constance, would we install the redis backend or database backend?

@jonespm
Copy link
Member

jonespm commented Nov 22, 2019

Just use the MySQL database backend that's already in the docker-compose and used for the application.

@jonespm
Copy link
Member

jonespm commented Jan 13, 2020

Here were my comments from #820 on how we might support this

I feel like for this to work as individual variables we'd have to either have a library that does type inference which is going to be a little more work than this (unless there's one out there).

Probably the easier way to fulfill the initial request I'd think is just to use this special ENV called
ENV_JSON

The thing with that is it's either or. You either have the entire env defined in ENV_JSON or you have it defined in the file. What I'd do instead of this.

  • Try to load The env from the file into ENV (on lines 32-38)
    8 Whether or not this passes or fails get the value ENV_JSON from the environment as ENV_JSON (lines 27-29)
  • Update the values from ENV_JSON into ENV. (You can just do this with ENV.update(ENV_JSON)
  • If ENV is still empty after all this, just set it to os.environ so it still runs.

This wouldn't allow someone to define individual values in the ENV and have them override but they could define a JSON in the env and it would override. I feel like this would be sufficient for the request and avoid modifying this entire file. Sorry as it looks like you did a a lot of work here.

@jonespm
Copy link
Member

jonespm commented May 5, 2020

@ssciolla I think you code from https://github.com/tl-its-umich-edu/course-inventory/blame/master/environ.py#L27 would solve this issue. Could you look at this when you get a chance? Maybe it'd be useful to refactor this over to a file like environ.py just like this instead of loading it in settings.

@ssciolla
Copy link
Contributor

@jonespm, yeah, I can start looking at this.

@ssciolla ssciolla added this to To do in MyLA-Default-Project via automation May 15, 2020
@jonespm
Copy link
Member

jonespm commented Jan 22, 2021

This relates to #1200

@jonespm jonespm added this to To do in MyLA-2021.03.01 Feb 18, 2021
@jennlove-um jennlove-um removed this from To do in MyLA-2021.03.01 May 12, 2021
@jennlove-um jennlove-um removed this from To do in MyLA-Default-Project Jul 6, 2022
@jennlove-um jennlove-um added this to To do in MyLA-2022.02.01 via automation Jul 6, 2022
@ssciolla ssciolla moved this from To do to In progress in MyLA-2022.02.01 Sep 7, 2022
MyLA-2022.02.01 automation moved this from In progress to Review/QA Sep 12, 2022
ssciolla added a commit that referenced this issue Sep 12, 2022
…1425)

* Add basic fix for environment variable override

* Remove GOOGLE_APPLICATION_CREDENTIALS from env_sample.hjson

* TZ -> TIME_ZONE; use TZ env var in settings as backup

* Add some info to configuration about overriding env.hjson values

* Extract overriding as a function; improve docs language

* Change log message to DEBUG level

* Fix bug

* Improve debug logging for clarity

* Remove unnecessary pass
@ssciolla ssciolla moved this from Review/QA to Review/QA - DEV in MyLA-2022.02.01 Sep 12, 2022
@ssciolla
Copy link
Contributor

ssciolla commented Sep 14, 2022

@jonespm, can you QA this when you have a minute? Test plan is on PR #1425.

@zqian
Copy link
Member

zqian commented Dec 8, 2022

I have followed the Test plan on PR #1425:

  • set "ROOT_LOG_LEVEL" to DEBUG in .env file
  • set different value for "TZ" param in env.hjson or env_udp.hjson file
  • verified the log message of ENV value for "TZ" overridden

@zqian zqian moved this from Review/QA - DEV to Done in MyLA-2022.02.01 Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📈 enhancement New feature or request
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

7 participants