Handle environment variables from yaml file
#3955
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3955 +/- ##
==========================================
- Coverage 63.66% 63.61% -0.05%
==========================================
Files 303 303
Lines 37968 38005 +37
Branches 2828 2835 +7
==========================================
+ Hits 24171 24178 +7
- Misses 13742 13772 +30
Partials 55 55 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Would rebase after merging #3956 to avoid issues locally (if changes are requested for this PR). |
| void create_empty_target( | ||
| const Context& context, | ||
| const fs::u8path& prefix, | ||
| const std::map<std::string, std::string>& env_vars, | ||
| bool no_env | ||
| ); |
There was a problem hiding this comment.
This will changes the API for mamba.
Must we do something specific (e.g. have default value for the new arguments) for it given that mamba has a specific policy for API and ABI stability?
There was a problem hiding this comment.
Answering to #3955 (comment):
Indeed, although this is in a detail namespace, this is visible in the ABI and this change is backward incompatible? We'ell have to update the ABI version accordingly (we can consider this change as API backward compatible since this method is not supposed to be used by clients).
On the long run, we should move all these detail functions in private headers (but this deserves a dedicated PR)
micromamba/tests/test_create.py
Outdated
| file_content.append( | ||
| "variables: {'MY_ENV_VAR': 'My Value', 'MY_OTHER_ENV_VAR': 'Another Value'}", | ||
| ) |
There was a problem hiding this comment.
Looking at this section from conda's documentation, it seems that is also a valid option which we must test:
variables:
MY_ENV_VAR: 'My Value'
MY_OTHER_ENV_VAR: 'Another Value'There was a problem hiding this comment.
Well it seems that we're using flow style everywhere in the tests when handling yaml files (I don't think the style changes anything functionally)...
But I changed to block style in b660b03 to have an example/reference.
| void create_empty_target( | ||
| const Context& context, | ||
| const fs::u8path& prefix, | ||
| const std::map<std::string, std::string>& env_vars, | ||
| bool no_env | ||
| ); |
There was a problem hiding this comment.
Answering to #3955 (comment):
Indeed, although this is in a detail namespace, this is visible in the ABI and this change is backward incompatible? We'ell have to update the ABI version accordingly (we can consider this change as API backward compatible since this method is not supposed to be used by clients).
On the long run, we should move all these detail functions in private headers (but this deserves a dedicated PR)
| @pytest.mark.parametrize("shared_pkgs_dirs", [True], indirect=True) | ||
| @pytest.mark.parametrize("env_vars", (False, True)) | ||
| @pytest.mark.parametrize("no_env", (False, True)) | ||
| def test_spec_file_env_vars(tmp_home, tmp_root_prefix, tmp_path, env_vars, no_env): |
There was a problem hiding this comment.
I tested locally, and activating the environment successfully sets environment variables.
(I wonder whether there's a test checking that environment variables are set when the environment is activated. I am checking for it.)
3 participants
Fix #3884
As specified in the CEP:
prefixis often ignored (it's the case incondaas well)platformsandcategoryare optional and are mostly used by tools that generate lockfiles.Therefore, those are not considered in
mamba.This PR handles the missing key
variablescorresponding to the environment variables to be set upon environment activation (throughstatefile).