feat: Add new command option to set env file for pdm script
Pull Request Checklist
- [x] A news fragment is added in
news/describing what is new. - [x] Test cases added for changed code.
Describe what you have changed in this PR.
Fix #3355
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 85.24%. Comparing base (
19f84aa) to head (cf0608f). Report is 12 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #3358 +/- ##
==========================================
+ Coverage 85.21% 85.24% +0.02%
==========================================
Files 112 112
Lines 11461 11472 +11
Branches 2505 2507 +2
==========================================
+ Hits 9767 9779 +12
+ Misses 1168 1167 -1
Partials 526 526
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 85.04% <100.00%> (+0.02%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
.env(effective for all environments).env.prod(effective forprodonly).env.local(effective for all environments).env.prod.local(effective forprodonly)The variables are merged in the above order, with the latter taking precedence.
The improvement is to make
.locala special modifier rather than a regular environment name. It contains the env vars for the current machine only, and.env*.localshould be added to the.gitignore, while other.envfiles can be shared safely.
Nice idea! I will update it ASAP
Sorry, I thought I answered this long ago, but it wasn't the case.
So here's my position:
I see value in the environment concept, but I consider that removing the env_file option from scripts is not a good idea (in my case I have dozen of corporate projects relying on this, with different values per script, the environment approach would not fit/scale those use cases). I also consider having an automatic env file loading a source of problems (you have no idea how much time I spent debugging just because an env file was silently loaded by a tool, and we were not aware of). So I would be against in the current form.
However, I have a counterproposal:
I see 2 main use cases for dotenv files:
- user dotenv (the classical
.env): not committed, used by devs to export their personal settings, credentials... Standard and often shared by multiple apps (like docker-compose) by default. Often managed by the devs in their own environment (direnv,mise,...) - project wide dotenvs, committed, tests and tooling may rely on it, this is most likely the kind of dotenv we will have in
env_filein the scripts.
I also like the idea of cascading env files.
So, I would:
- support a opt-in user dotenv autoloading, with a setting, either a boolean to force the standard
.env, either a configurable string (but it is important for people having their own dotenv loader likedirenvormiseto avoid forcing them into a double loading, most probably with different capabilities) - support the environment concept with the
PDM_ENVIRONMENTenv var and the-e/--environmentwith default onlocallike done in this PR (think that devs may want to setPDM_ENVIRONMENTin their.env, it should be loaded first if the option is enabled) - make the
env_filesupport a{environment}placeholder so it is possible to writetask.env_file = ".env.{environment}"(or_.env_fileto make it global) while keeping backward compatibility - make
env_filealso support a list ofenv_fileso it is possible to have cascading (it should also be failsafe so it is possible to list optional dotenvs the user may add or not)
Then this should work:
_.env_file = [
".env.{environment}",
".env.{environment}.local",
]
But also this:
my-task.env_file = [
"dotenvs/my-task",
"dotenvs/my-task.{environment}",
"dotenvs/my-task.{environment}.local",
]
Bonus: the environment could also be exposed as script placeholder
What do you think ?