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.

Sign up for GitHub

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

Jump to bottom

Use env var to disable loading dotenv #2724

Merged
merged 2 commits into from
Apr 29, 2018
Merged

Use env var to disable loading dotenv #2724

merged 2 commits into from
Apr 29, 2018

Conversation

lepture
Copy link
Contributor

@lepture lepture commented Apr 26, 2018

Fix issue: #2722

@lepture
Copy link
Contributor Author

lepture commented Apr 26, 2018

I need to add a test case.

ThiefMaster
ThiefMaster reviewed Apr 26, 2018
View reviewed changes
flask/cli.py Outdated
@@ -544,7 +544,10 @@ def main(self, *args, **kwargs):
# script that is loaded here also attempts to start a server.
os.environ['FLASK_RUN_FROM_CLI'] = 'true'

if self.load_dotenv:
val = os.environ.get('FLASK_DONT_LOAD_ENV')
load_dotenv = not val or val in ('0', 'false', 'no')

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

@lepture
Copy link
Contributor Author

lepture commented Apr 26, 2018

Oops. Tests should not fail since I didn't add any changes to break the test. Weird.

@lepture
Copy link
Contributor Author

lepture commented Apr 27, 2018

@davidism any idea why tests failed? I've tested master branch on my local machine, tests failed too.

@davidism
Copy link
Member

I can take a look tomorrow. The most common issue I had with CLI tests was that a test would forget to reset the env vars. Not sure if that's related here.

@lepture
Copy link
Contributor Author

lepture commented Apr 27, 2018

The weird thing is master branch's tests fail on my local machine.

@davidism davidism added this to the 1.0.1 milestone Apr 27, 2018
@davidism davidism added the cli label Apr 27, 2018
@davidism davidism changed the base branch from master to 1.0-maintenance April 28, 2018 14:19
@davidism
Copy link
Member

Working on this. Changed base to 1.0-maintenance.

davidism
davidism reviewed Apr 28, 2018
View reviewed changes
flask/cli.py Outdated
@@ -544,7 +544,10 @@ def main(self, *args, **kwargs):
# script that is loaded here also attempts to start a server.
os.environ['FLASK_RUN_FROM_CLI'] = 'true'

if self.load_dotenv:
val = os.environ.get('FLASK_DONT_LOAD_ENV')
load_dotenv = not val or val in ('0', 'false', 'no')

This comment was marked as off-topic.

Sign in to view

@davidism
Copy link
Member

Needs a test.

@rhymes
Copy link

rhymes commented Apr 28, 2018 •
edited
Loading

@davidism

Shouldn't it be called "FLASK_IGNORE_DOTENV" or "FLASK_SKIP_DOTENV" ?

"Flask don't load env" it's not exactly accurate because the flag purpose is to skip reading variables from .env, not to skip the shell environment.

@davidism
Copy link
Member

Discussion at #2722, it was chosen to match pipenv's PIPENV_DONT_LOAD_ENV. I agree that either of those sound better though; I like SKIP.

@davidism davidism changed the title Use FLASK_DONT_LOAD_ENV flag to disable load .env Use env var to disable loading dotenv Apr 28, 2018
@davidism davidism merged commit 6f84cf4 into 1.0-maintenance Apr 29, 2018
@davidism davidism deleted the issue-2722 branch April 29, 2018 21:27
@davidism
Copy link
Member

Added the check to app.run as well in 6e1e3e0.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@davidism davidism davidism left review comments

@ThiefMaster ThiefMaster ThiefMaster left review comments

Assignees
No one assigned
Labels
cli
Projects
None yet
Milestone
1.0.1
Development

Successfully merging this pull request may close these issues.
4 participants
@lepture @davidism @rhymes @ThiefMaster