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 support for .env and custom env files in uv run #8263

Merged
merged 9 commits into from
Oct 22, 2024

Conversation

edugzlez
Copy link
Contributor

Summary

I have been reading discussion #1384 about .env and how to include it in the uv run command.

I have always wanted to include this possibility in uv, so I was interested in the latest changes. Following @charliermarsh's advice I have tried to respect the philosophy that uv run uses the default .env and this can be discarded or changed via terminal or environment variables.

The behaviour is as follows:

  • uv run file.py executes file.py using the .env (if it exists).
  • uv run --env-file .env.development file.py uses the .env.development file to load the variables and then runs file.py. In this case the program fails if the file does not exist.
  • uv run --no-env-file file.py skips reading the .env file.

Equivalently, I have included the UV_ENV_FILE and UV_NO_ENV_FILE environment variables.

Test Plan

I haven't got into including automated tests, I would need help with this.

I have tried the above commands, with a python script that prints environment variables.

@zanieb zanieb requested a review from charliermarsh October 16, 2024 16:54
@charliermarsh charliermarsh self-assigned this Oct 16, 2024
@charliermarsh charliermarsh added this to the v0.5.0 milestone Oct 16, 2024
@charliermarsh
Copy link
Member

This generally looks good, but I think we should bundle it in v0.5.0, since it is a change in behavior.

@charliermarsh
Copy link
Member

Do you mind adding a test to run.rs too, to validate that the files are being loaded and passed to the runtime?

@edugzlez
Copy link
Contributor Author

edugzlez commented Oct 16, 2024

Thank you very much for your advice and input.

The coding experience when writing tests is fantastic, I didn't expect it to be so good.

There seems to be a problem running the tests, but it escapes my understanding. I've seen that other PRs are having the same error.

// Initialize env file, if necessary.
if !no_env_file {
let env_file_path = env_file
.clone()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this clone is not necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is necessary because you then need to use env_file to check whether it is passed as an argument or the default argument is taken.

if env_file.is_some() {

Maybe I could change it to this:

let is_env_file_present = env_file.is_some();
let env_file_path = env_file.unwrap_or_else(|| Path::new(".env").to_path_buf());

let loaded = dotenvy::from_path_override(&env_file_path);

match loaded {
    Err(dotenvy::Error::Io(err)) => {
        if is_env_file_present {
            bail!(
                "Failed to read environment file `{}`: {}",
                env_file_path.display(),
                err
            );
        }
    }

to avoid the clone (but at the cost of creating a variable first).

I'm not sure.

Copy link
Member

Choose a reason for hiding this comment

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

I was able to remove the clone, if you're interested in seeing how.

@charliermarsh
Copy link
Member

We're planning to do v0.5 next week (hopefully) which would include this.

@charliermarsh charliermarsh changed the base branch from main to tracking/050 October 22, 2024 02:24
@charliermarsh
Copy link
Member

Will get this merged into the v0.5 branch tomorrow.

@charliermarsh charliermarsh force-pushed the feat/uv-run-env-file branch 4 times, most recently from b02fbdf to 8b6bbb0 Compare October 22, 2024 14:01
@charliermarsh charliermarsh merged commit f6d9804 into astral-sh:tracking/050 Oct 22, 2024
62 of 63 checks passed
@charliermarsh
Copy link
Member

It's really common for runners to support .env files -- Node does it, Bun does it, Deno does it. If you don't want the feature to be enabled, you can just opt out of it!

@AndreuCodina
Copy link

AndreuCodina commented Oct 22, 2024

I'm not a NodeJS developer, but this PR is breaking conventions and good practices.

Developers always need to load the .env.Local values when they execute the code in localhost. Why do they have to use uv run --env-file .env.Local instead of just uv run? It doesn't make sense to me, even if it's optional.

But also, the code loses the control to manage the sources, their priority and their order because the runner tool decided to load values from a configuration file by default.

@charliermarsh
Copy link
Member

Developers always need to load the .env.Local values when they execute the code in localhost. Why do they have to use uv run --env-file .env.Local instead of just uv run? It doesn't make any sense for me.

I don't understand this critique. Are you asking for uv run to automatically load anything that's prefixed with .env? Or are you asking that uv run never detect these?

But also, the code loses the control to manage the sources, their priority and their order because the runner tool decided to load values from a configuration file by default.

Again: I'm sorry you feel that way but you can just disable this feature if you don't want it. A more reasonable critique would be to say that this should be opt-in rather than opt-out.

@AndreuCodina
Copy link

AndreuCodina commented Oct 22, 2024

I don't understand this critique. Are you asking for uv run to automatically load anything that's prefixed with .env? Or are you asking that uv run never detect these?

Never. Imagine this case:

You have three environments (.env.Development, .env.Staging, .env.Production) and optionally localhost (.env.Local). pydantic-settings reads the .env file to get shared settings, and then read an environment variable (to detect the environment name) to load the proper .env file (.env.Local for example).

Another example: in .NET, by default, the environment variables have priority over configuration files (https://learn.microsoft.com/en-us/aspnet/core/fundamentals/configuration/?view=aspnetcore-8.0#default-application-configuration-sources). At this point, this PR is mixing 2 sources into one, and they are different.

When you develop a backend, you must read from several sources, and therefore you need pydantic-settings or another library. You have to load secrets from Azure Key Vault (they can't be in configuration files), auto-reloadable configurations from Azure App Configuration, configuration files, environment variables, etc.

So, you have to load from several sources.

What's an example of a shared configuration? The number of messages to read from a queue, the server to log in against, the timeout of a webhook... And sometimes you need to override it, for example, modifying the App Service environment variable, Dockerfile, App Configuration...

This PR adds the capability of, by default, reading from a source (a configuration file) and loading its values into another source (the environment variables).

Again: I'm sorry you feel that way but you can just disable this feature if you don't want it. A more reasonable critique would be to say that this should be opt-in rather than opt-out.

I'm against adding this because instead of advantages or options, it adds a feature that I'm pretty sure how it'll be used: instead of adding a library such as pydantic-settings, developers will add parameters to uv to read from configuration files, and they'll load the secrets modifying the cloud, adding environment variables by hand.

But yes, my issue is uv is reading from the .env file by default when I think it shouldn't.

zanieb pushed a commit that referenced this pull request Oct 22, 2024
I have been reading discussion #1384 about .env and how to include it in
the `uv run` command.

I have always wanted to include this possibility in `uv`, so I was
interested in the latest changes. Following @charliermarsh's [advice
](#1384 (comment)) I
have tried to respect the philosophy that `uv run` uses the default
`.env` and this can be discarded or changed via terminal or environment
variables.

The behaviour is as follows:
- `uv run file.py` executes file.py using the `.env` (if it exists).
- `uv run --env-file .env.development file.py` uses the
`.env.development` file to load the variables and then runs file.py. In
this case the program fails if the file does not exist.
- `uv run --no-env-file file.py` skips reading the `.env` file.

Equivalently, I have included the `UV_ENV_FILE` and `UV_NO_ENV_FILE`
environment variables.

I haven't got into including automated tests, I would need help with
this.

I have tried the above commands, with a python script that prints
environment variables.

---------

Co-authored-by: Charlie Marsh <[email protected]>
@pantheraleo-7
Copy link
Contributor

A more reasonable critique would be to say that this should be opt-in rather than opt-out.

This (reading .env file) is a great feature, but not a sensible default. It should be opt-in.

If the environment is not loaded programmatically, uv run doing it (by default, without any flags) may cause different behaviour when running it without uv and also it will be too much "magic BTS".

Explicit is better than implicit.
~ Zen of Python, PEP20

@zanieb zanieb mentioned this pull request Oct 22, 2024
@seapagan
Copy link

A more reasonable critique would be to say that this should be opt-in rather than opt-out.

This (reading .env file) is a great feature, but not a sensible default. It should be opt-in.

If the environment is not loaded programmatically, uv run doing it (by default, without any flags) may cause different behaviour when running it without uv and also it will be too much "magic BTS".

Explicit is better than implicit.

~ Zen of Python, PEP20

Opt-in as it's technically not backwards compatible and may be unexpected - and I'd argue the majority of users won't use this. In production my apps may need to read .env vars (though should be done by actual vars at that stage) but they are never RUN through 'uv'. So they need to include dotenv or whatever anyway.

My big issue is transparency- some env vars can change the actual paths and options of that process - I don't want to run 'uv' in a repo I've just cloned and have it QUIETLY set env vars I don't know about.

Opt in please. Make it a global setting that is set and forget, but please don't do it behind our backs.

@charliermarsh
Copy link
Member

(Not disagreeing with your broader point, but I'll just note that this is already marked as a breaking change that would be included in v0.5 per our versioning policy. It's ok for it not to be backwards compatible.)

@charliermarsh charliermarsh added breaking A breaking change enhancement New feature or improvement to existing functionality labels Oct 22, 2024
@seapagan
Copy link

Fair point 😃

My points still stand though. There is a reason that 'direnv' REQUIRES you to physically allow auto-reading an env file the first time - security.

Maybe do this for each new project the first time? At least give the user the choice. Otherwise a global option, but please, Opt in.

@zanieb
Copy link
Member

zanieb commented Oct 22, 2024

My big issue is transparency- some env vars can change the actual paths and options of that process - I don't want to run 'uv' in a repo I've just cloned and have it QUIETLY set env vars I don't know about.

I don't know if this is a fair concern. If you clone an arbitrary repository and execute uv run in it, you're already trusting the repository. They can execute arbitrary code at that point. direnv is different because it hooks into your shell and affects any command you run in the directory (i.e. the repository can be untrusted and you can still operate on it).

@seapagan
Copy link

I don't know if this is a fair concern. If you clone an arbitrary repository and execute uv run in it, you're already trusting the repository. They can execute arbitrary code at that point. direnv is different because it hooks into your shell and affects any command you run in the directory (i.e. the repository can be untrusted and you can still operate on it).

This is very true yes. Maybe it's just my prejudice showing, and I really have no use-case for it. In many cases my code is running uvicorn-> guniciorn -> nginx so 'uv' is not in the pipe and any .env would need to be loaded manually. If I'm using an env file, I'd make sure my code explicitly reads it, and that is really more Zen.

I truly believe you may end up with new issues related to unexpected env vars being set - how many people really read the release notes or just go 'yeah yeah, upgrade'😜.

I'll stand by my 'opt-in' recommendation but obv not going to stop using uv if you choose otherwise.

@seapagan
Copy link

One more thing - anyone who wants and uses this feature should have no problem opting in?🤷‍♂️

@TheRealBecks
Copy link

After the migration from pipenv to uv our local Ansible environments are broken as uv does not read the .env file by default.

Also Docker and Ansible read .env by default.

In my case (what our company does): The .env files are only needed for local development where users also run uv run ... manually. Opt-in would be no comfort from the users perspective especially when projects will be migrated from tools like pipenv to uv. Also production systems get different environment variables that will be set for the run and will not use any .env file and if I would like to change that behaviour I will use the UV_xx environment variables.

So, maybe this approach will be accepted:

  1. Use opt-out by default, so uv reads the .env file by default
  2. Add UV_xx environment variables for not reading and reading .env files: You already planned this feature
  3. Add another option for the pyproject.toml file so reading an .env file for a project can be set to opt-in instead of the default opt-out

@seapagan @AndreuCodina Would option 3 something you would like to see in uv?

zanieb pushed a commit that referenced this pull request Oct 24, 2024
I have been reading discussion #1384 about .env and how to include it in
the `uv run` command.

I have always wanted to include this possibility in `uv`, so I was
interested in the latest changes. Following @charliermarsh's [advice
](#1384 (comment)) I
have tried to respect the philosophy that `uv run` uses the default
`.env` and this can be discarded or changed via terminal or environment
variables.

The behaviour is as follows:
- `uv run file.py` executes file.py using the `.env` (if it exists).
- `uv run --env-file .env.development file.py` uses the
`.env.development` file to load the variables and then runs file.py. In
this case the program fails if the file does not exist.
- `uv run --no-env-file file.py` skips reading the `.env` file.

Equivalently, I have included the `UV_ENV_FILE` and `UV_NO_ENV_FILE`
environment variables.

I haven't got into including automated tests, I would need help with
this.

I have tried the above commands, with a python script that prints
environment variables.

---------

Co-authored-by: Charlie Marsh <[email protected]>
@zanieb zanieb mentioned this pull request Oct 24, 2024
@seapagan
Copy link

I'd still ask kindly if you can add a setting to the uv.toml so we can disable this globally please 🙏

zanieb pushed a commit that referenced this pull request Oct 25, 2024
I have been reading discussion #1384 about .env and how to include it in
the `uv run` command.

I have always wanted to include this possibility in `uv`, so I was
interested in the latest changes. Following @charliermarsh's [advice
](#1384 (comment)) I
have tried to respect the philosophy that `uv run` uses the default
`.env` and this can be discarded or changed via terminal or environment
variables.

The behaviour is as follows:
- `uv run file.py` executes file.py using the `.env` (if it exists).
- `uv run --env-file .env.development file.py` uses the
`.env.development` file to load the variables and then runs file.py. In
this case the program fails if the file does not exist.
- `uv run --no-env-file file.py` skips reading the `.env` file.

Equivalently, I have included the `UV_ENV_FILE` and `UV_NO_ENV_FILE`
environment variables.

I haven't got into including automated tests, I would need help with
this.

I have tried the above commands, with a python script that prints
environment variables.

---------

Co-authored-by: Charlie Marsh <[email protected]>
zanieb added a commit that referenced this pull request Oct 25, 2024
I have been reading discussion #1384 about .env and how to include it in
the `uv run` command.

I have always wanted to include this possibility in `uv`, so I was
interested in the latest changes. Following @charliermarsh's [advice
](#1384 (comment)) I
have tried to respect the philosophy that `uv run` uses the default
`.env` and this can be discarded or changed via terminal or environment
variables.

The behaviour is as follows:
- `uv run file.py` executes file.py using the `.env` (if it exists).
- `uv run --env-file .env.development file.py` uses the
`.env.development` file to load the variables and then runs file.py. In
this case the program fails if the file does not exist.
- `uv run --no-env-file file.py` skips reading the `.env` file.

Equivalently, I have included the `UV_ENV_FILE` and `UV_NO_ENV_FILE`
environment variables.

I haven't got into including automated tests, I would need help with
this.

I have tried the above commands, with a python script that prints
environment variables.

---------

Co-authored-by: Charlie Marsh <[email protected]>
@artificial-aidan
Copy link

Suggestion here to make the Read .env file debug output info or something else visible more easily. It will help with the transition, and prevent confusion of people not realizing the silent behavior.

charliermarsh added a commit that referenced this pull request Oct 28, 2024
I have been reading discussion #1384 about .env and how to include it in
the `uv run` command.

I have always wanted to include this possibility in `uv`, so I was
interested in the latest changes. Following @charliermarsh's [advice
](#1384 (comment)) I
have tried to respect the philosophy that `uv run` uses the default
`.env` and this can be discarded or changed via terminal or environment
variables.

The behaviour is as follows:
- `uv run file.py` executes file.py using the `.env` (if it exists).
- `uv run --env-file .env.development file.py` uses the
`.env.development` file to load the variables and then runs file.py. In
this case the program fails if the file does not exist.
- `uv run --no-env-file file.py` skips reading the `.env` file.

Equivalently, I have included the `UV_ENV_FILE` and `UV_NO_ENV_FILE`
environment variables.

I haven't got into including automated tests, I would need help with
this.

I have tried the above commands, with a python script that prints
environment variables.

---------

Co-authored-by: Charlie Marsh <[email protected]>
zanieb added a commit that referenced this pull request Oct 29, 2024
I have been reading discussion #1384 about .env and how to include it in
the `uv run` command.

I have always wanted to include this possibility in `uv`, so I was
interested in the latest changes. Following @charliermarsh's [advice
](#1384 (comment)) I
have tried to respect the philosophy that `uv run` uses the default
`.env` and this can be discarded or changed via terminal or environment
variables.

The behaviour is as follows:
- `uv run file.py` executes file.py using the `.env` (if it exists).
- `uv run --env-file .env.development file.py` uses the
`.env.development` file to load the variables and then runs file.py. In
this case the program fails if the file does not exist.
- `uv run --no-env-file file.py` skips reading the `.env` file.

Equivalently, I have included the `UV_ENV_FILE` and `UV_NO_ENV_FILE`
environment variables.

I haven't got into including automated tests, I would need help with
this.

I have tried the above commands, with a python script that prints
environment variables.

---------

Co-authored-by: Charlie Marsh <[email protected]>
zanieb added a commit that referenced this pull request Nov 4, 2024
I have been reading discussion #1384 about .env and how to include it in
the `uv run` command.

I have always wanted to include this possibility in `uv`, so I was
interested in the latest changes. Following @charliermarsh's [advice
](#1384 (comment)) I
have tried to respect the philosophy that `uv run` uses the default
`.env` and this can be discarded or changed via terminal or environment
variables.

The behaviour is as follows:
- `uv run file.py` executes file.py using the `.env` (if it exists).
- `uv run --env-file .env.development file.py` uses the
`.env.development` file to load the variables and then runs file.py. In
this case the program fails if the file does not exist.
- `uv run --no-env-file file.py` skips reading the `.env` file.

Equivalently, I have included the `UV_ENV_FILE` and `UV_NO_ENV_FILE`
environment variables.

I haven't got into including automated tests, I would need help with
this.

I have tried the above commands, with a python script that prints
environment variables.

---------

Co-authored-by: Charlie Marsh <[email protected]>
charliermarsh added a commit that referenced this pull request Nov 4, 2024
I have been reading discussion #1384 about .env and how to include it in
the `uv run` command.

I have always wanted to include this possibility in `uv`, so I was
interested in the latest changes. Following @charliermarsh's [advice
](#1384 (comment)) I
have tried to respect the philosophy that `uv run` uses the default
`.env` and this can be discarded or changed via terminal or environment
variables.

The behaviour is as follows:
- `uv run file.py` executes file.py using the `.env` (if it exists).
- `uv run --env-file .env.development file.py` uses the
`.env.development` file to load the variables and then runs file.py. In
this case the program fails if the file does not exist.
- `uv run --no-env-file file.py` skips reading the `.env` file.

Equivalently, I have included the `UV_ENV_FILE` and `UV_NO_ENV_FILE`
environment variables.

I haven't got into including automated tests, I would need help with
this.

I have tried the above commands, with a python script that prints
environment variables.

---------

Co-authored-by: Charlie Marsh <[email protected]>
charliermarsh added a commit that referenced this pull request Nov 4, 2024
## Summary

This PR pulls in #8263 and
#8463, which were originally merged
into the v0.5 tracking branch but can now be committed separately, as
we've made `.env` loading opt-in.

In summary:

- `.env` loading is now opt-in (`--env-file .env`).
- `.env` remains supported on `uv run`, so it's meant for providing
environment variables to the run command, rather than to uv itself.

---------

Co-authored-by: Eduardo González Vaquero <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking A breaking change enhancement New feature or improvement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants