-
Notifications
You must be signed in to change notification settings - Fork 808
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 load from .env files before invoking the command #7357
add ability to load from .env files before invoking the command #7357
Conversation
f13a8eb
to
6279982
Compare
|
@eth3lbert I will switch it to dotenvy if you are happy with the idea of the PR :) |
36af9a3
to
7818e55
Compare
3bf5307
to
3c49806
Compare
Can't wait for this to land, we're excited to switch to uv, but are currently blocked by this. |
a35c096
to
2cdbd23
Compare
@charliermarsh This PR is currently configured to be opt in, but in light of the discussion in the issue thread, should we instead make it opt out? i.e.
And also the naming can be discussed, I kept it scoped to |
@PhilipVinc currently it only affects the |
// to ensure they are not overwritten | ||
if load_dotenv { | ||
debug!("Loading `.env` file"); | ||
dotenv_override().ok(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently this will just fail silently if the file is malformed or missing - is this what we want?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO missing shouldn't show an error, because the chance that the file is missing means that someone doesn't want to use .env is high.
However, a malformed file should be indicated to the developer. The chance that they have intentionally created a .env file that is malformed, in order for it not to be loaded by uv, is very low. I would suggest a warning rather than an error, because execution can still continue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we should show a user-facing warning if there's an error other than "file does not exist". We could also show a warning if it's "file does not exist" and the user provided an explicit path to a dotenv file, but I'm not certain if we should and it's not blocking.
Yeah I think my preference would be:
I think we can omit the |
closing this as it seems to be implemented already |
Summary
Pipenv and other equivalent tools have the ability to load an .env file during execution of a script etc
https://pipenv.pypa.io/en/stable/shell.html#automatic-loading-of-env
The PR adds a new flag
--load-dotenv
and a new ENVUV_RUN_LOAD_DOTENV
that enables this feature foruv run
resolves #1384
Test Plan
Have tested it locally by successfully invoking scripts that require ENV's to be set.