Repository: valohai/valohai-cli

Issue:
CLI does not honor default environment variables in step
They're probably currently only populated into the GUI.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/valohai/valohai-cli/pull/85?src=pr&el=h1) Report
> Merging [#85](https://codecov.io/gh/valohai/valohai-cli/pull/85?src=pr&el=desc) into [master](https://codecov.io/gh/valohai/valohai-cli/commit/e75957cdb64b2ef5af39cea755712334cc72c42e?src=pr&el=desc) will **decrease** coverage by `0.25%`.
> The diff coverage is `0%`.

[![Impacted file tree graph](https://codecov.io/gh/valohai/valohai-cli/pull/85/graphs/tree.svg?width=650&token=xqgKRx94XH&height=150&src=pr)](https://codecov.io/gh/valohai/valohai-cli/pull/85?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master      #85      +/-   ##
==========================================
- Coverage   86.48%   86.23%   -0.26%     
==========================================
  Files          95       95              
  Lines        2730     2680      -50     
  Branches      379      374       -5     
==========================================
- Hits         2361     2311      -50     
  Misses        238      238              
  Partials      131      131
```


| [Impacted Files](https://codecov.io/gh/valohai/valohai-cli/pull/85?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...\_cli/commands/execution/run/dynamic\_run\_command.py](https://codecov.io/gh/valohai/valohai-cli/pull/85/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvY29tbWFuZHMvZXhlY3V0aW9uL3J1bi9keW5hbWljX3J1bl9jb21tYW5kLnB5) | `77.66% <0%> (-2.34%)` | :arrow_down: |
| [valohai\_cli/commands/environments.py](https://codecov.io/gh/valohai/valohai-cli/pull/85/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvY29tbWFuZHMvZW52aXJvbm1lbnRzLnB5) | `35.29% <0%> (-17.65%)` | :arrow_down: |
| [valohai\_cli/table.py](https://codecov.io/gh/valohai/valohai-cli/pull/85/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvdGFibGUucHk=) | `81.13% <0%> (-7.25%)` | :arrow_down: |
| [valohai\_cli/commands/execution/outputs.py](https://codecov.io/gh/valohai/valohai-cli/pull/85/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvY29tbWFuZHMvZXhlY3V0aW9uL291dHB1dHMucHk=) | `64.86% <0%> (-1.81%)` | :arrow_down: |
| [valohai\_cli/commands/execution/list.py](https://codecov.io/gh/valohai/valohai-cli/pull/85/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvY29tbWFuZHMvZXhlY3V0aW9uL2xpc3QucHk=) | `75% <0%> (-1.2%)` | :arrow_down: |
| [tests/fixture\_data.py](https://codecov.io/gh/valohai/valohai-cli/pull/85/diff?src=pr&el=tree#diff-dGVzdHMvZml4dHVyZV9kYXRhLnB5) | `100% <0%> (ø)` | :arrow_up: |
| [valohai\_cli/yaml\_wizard.py](https://codecov.io/gh/valohai/valohai-cli/pull/85/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkveWFtbF93aXphcmQucHk=) | `87.03% <0%> (ø)` | :arrow_up: |
| [tests/commands/execution/test\_outputs.py](https://codecov.io/gh/valohai/valohai-cli/pull/85/diff?src=pr&el=tree#diff-dGVzdHMvY29tbWFuZHMvZXhlY3V0aW9uL3Rlc3Rfb3V0cHV0cy5weQ==) | `100% <0%> (ø)` | :arrow_up: |
| [tests/commands/execution/utils.py](https://codecov.io/gh/valohai/valohai-cli/pull/85/diff?src=pr&el=tree#diff-dGVzdHMvY29tbWFuZHMvZXhlY3V0aW9uL3V0aWxzLnB5) | `100% <0%> (ø)` | :arrow_up: |
| [valohai\_cli/commands/init.py](https://codecov.io/gh/valohai/valohai-cli/pull/85/diff?src=pr&el=tree#diff-dmFsb2hhaV9jbGkvY29tbWFuZHMvaW5pdC5weQ==) | `78.26% <0%> (+8.26%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/valohai/valohai-cli/pull/85?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/valohai/valohai-cli/pull/85?src=pr&el=footer). Last update [e75957c...12f734b](https://codecov.io/gh/valohai/valohai-cli/pull/85?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/valohai_cli/commands/execution/run/dynamic_run_command.py b/valohai_cli/commands/execution/run/dynamic_run_command.py
index 38cf466..02eae5f 100644
--- a/valohai_cli/commands/execution/run/dynamic_run_command.py
+++ b/valohai_cli/commands/execution/run/dynamic_run_command.py
@@ -80,6 +80,9 @@ class RunCommand(click.Command):
             self.params.append(self.convert_param_to_option(parameter))
         for input in step.inputs.values():
             self.params.append(self.convert_input_to_option(input))
+        for name, value in step.environment_variables.items():
+            if name not in self.environment_variables:
+                self.environment_variables[name] = value.default
 
     def convert_param_to_option(self, parameter):
         """
diff --git a/valohai_cli/utils/__init__.py b/valohai_cli/utils/__init__.py
index 68053a8..4ba3f90 100644
--- a/valohai_cli/utils/__init__.py
+++ b/valohai_cli/utils/__init__.py
@@ -184,10 +184,6 @@ def parse_environment_variable_strings(envvar_strings):
     environment_variables = {}
     for string in envvar_strings:
         key, _, value = string.partition('=')
-        if not value:
-            raise ValueError('Environment variable specification {string} must be in the format KEY=VALUE'.format(
-                string=string,
-            ))
         key = key.strip()
         if not key:
             continue


Test patch:
diff --git a/tests/commands/execution/test_run.py b/tests/commands/execution/test_run.py
index 89b66c1..fd9e5ea 100644
--- a/tests/commands/execution/test_run.py
+++ b/tests/commands/execution/test_run.py
@@ -98,7 +98,7 @@ def test_run_requires_step(runner, logged_in_and_linked):
 @pytest.mark.parametrize('pass_param', (False, True))
 @pytest.mark.parametrize('pass_input', (False, True))
 @pytest.mark.parametrize('pass_env', (False, True))
-@pytest.mark.parametrize('pass_env_var', (False, True))
+@pytest.mark.parametrize('pass_env_var', (None, 'custom', 'override-default'))
 @pytest.mark.parametrize('adhoc', (False, True), ids=('regular', 'adhoc'))
 def test_run(runner, logged_in_and_linked, monkeypatch, pass_param, pass_input, pass_env, pass_env_var, adhoc):
     project_id = PROJECT_DATA['id']
@@ -130,10 +130,16 @@ def test_run(runner, logged_in_and_linked, monkeypatch, pass_param, pass_input,
         args.extend(['-v', 'greeting=hello'])
         args.extend(['--var', 'enable=1'])
         args.extend(['-vdebug=yes'])
+        if pass_env_var == 'override-default':
+            args.extend(['--var', 'testenvvar='])
+            expected_testenvvar = ''
+        else:
+            expected_testenvvar = 'test'  # default from YAML
         values['environment_variables'] = {
             'greeting': 'hello',
             'enable': '1',
             'debug': 'yes',
+            'testenvvar': expected_testenvvar,
         }
     with RunAPIMock(project_id, commit_id, values):
         output = runner.invoke(run, args, catch_exceptions=False).output
diff --git a/tests/fixture_data.py b/tests/fixture_data.py
index b8f4fa6..d013318 100644
--- a/tests/fixture_data.py
+++ b/tests/fixture_data.py
@@ -121,6 +121,9 @@ CONFIG_YAML = """
         description: Number of steps to run the trainer
         type: integer
         default: 300
+    environment-variables:
+      - name: testenvvar
+        default: 'test'
 
 """