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

Add ability to read config variables from env #933

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

altexdim
Copy link

@altexdim altexdim commented Sep 9, 2021

Add ability to read config variables from env

Usage example:

DONKEYCAR_CFG_MAX_LOOPS=2000 python manage.py drive

this command will override config variable MAX_LOOPS and set it to 2000
this is the equivalent of having MAX_LOOPS=2000 in the myconfig.py

Copy link
Contributor

@DocGarbanzo DocGarbanzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great addition. Can you please also add a small test? Afterwards, please rebase (might do nothing...) and squash your commits into one, then I'll merge this.

@@ -51,19 +61,20 @@ def load_config(config_path=None, myconfig="myconfig.py"):
if os.path.exists(local_config):
config_path = local_config

print('loading config file: {}'.format(config_path))
logger = getLogger()
logger.info('loading config file: {}' . format(config_path))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, we should be moving to logging. Can you please also use f-strings?

@DocGarbanzo
Copy link
Contributor

@altexdim - do you want to address the comments so we can merge this?

@altexdim
Copy link
Author

Yes, absolutely, thank you for the review, I'll fix it as I find some time

@Ezward
Copy link
Contributor

Ezward commented Oct 26, 2022

@DocGarbanzo if we like this feature (I like it), we should change that to f-string ourselves or merge as is.

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

Successfully merging this pull request may close these issues.

3 participants