Add support for env files. #2296
Conversation
… and be passed on to override any yaml values
sky/cli.py
Outdated
| env: List[Tuple[str, str]], | ||
| env_file: Optional[Dict[str, str]], |
There was a problem hiding this comment.
| env: List[Tuple[str, str]], | |
| env_file: Optional[Dict[str, str]], | |
| env_file: Optional[Dict[str, str]], | |
| env: List[Tuple[str, str]], |
nit: Would be nice to keep the order in _TASK_OPTIONS and here to be the same.
| @@ -1424,6 +1447,7 @@ def exec( | |||
| use_spot: Optional[bool], | |||
| image_id: Optional[str], | |||
| env: List[Tuple[str, str]], | |||
| env_file: Optional[Dict[str, str]], | |||
| @@ -3460,6 +3485,7 @@ def spot_launch( | |||
| image_id: Optional[str], | |||
| spot_recovery: Optional[str], | |||
| env: List[Tuple[str, str]], | |||
| env_file: Optional[Dict[str, str]], | |||
| @@ -3908,6 +3935,7 @@ def benchmark_launch( | |||
| use_spot: Optional[bool], | |||
| image_id: Optional[str], | |||
| env: List[Tuple[str, str]], | |||
| env_file: Optional[Dict[str, str]], | |||
|
BTW, you could use pip install -r requirements-dev.txt
./format.shto auto-format your code to pass the pylint and format check 🙂 |
There was a problem hiding this comment.
Thanks a bunch @fozziethebeat. Works like a charm. Tested
sky launch --cloud aws --use-spot --cpus 1+ --down --env-file=.env -- 'echo $HI $SKY'
with a manually written .env file.
Minor (not needed): it may be good to add a sky exec --env-file test here:
https://github.com/skypilot-org/skypilot/blob/master/tests/test_smoke.py#L2189.
sky/cli.py
Outdated
| @@ -25,6 +25,7 @@ | |||
| """ | |||
| import copy | |||
| import datetime | |||
| from dotenv import dotenv_values | |||
There was a problem hiding this comment.
nit: we use google styleguide where 3rd party libs are imported in 2nd block, L42-45. Also from styleguide: import dotenv and use dotenv.dotenv_values below?
sky/cli.py
Outdated
| click.option( | ||
| '--env-file', | ||
| required=False, | ||
| type=dotenv_values, |
There was a problem hiding this comment.
nit: dotenv.dotenv_values
sky/setup_files/setup.py
Outdated
| @@ -86,6 +86,7 @@ def parse_readme(readme: str) -> str: | |||
| # PrettyTable with version >=2.0.0 is required for the support of | |||
| # `add_rows` method. | |||
| 'PrettyTable>=2.0.0', | |||
| 'python-dotenv==1.0.0', | |||
There was a problem hiding this comment.
Can we relax this? This requires >= 3.8. Even though 3.7 is end of life now, it seems removing this will install 0.21.1 and it still works for the use case (tested).
» pip install python-dotenv==1.0.0 130 ↵
ERROR: Ignored the following versions that require a different python version: 1.0.0 Requires-Python >=3.8
ERROR: Could not find a version that satisfies the requirement python-dotenv==1.0.0 (from versions: 0.1.0, 0.1.2, 0.1.3, 0.1.5, 0.2.0, 0.3.0, 0.4.0, 0.5.0, 0.5.1, 0.6.0, 0.6.1, 0.6.2, 0.6.3, 0.6.4, 0.6.5, 0.7.0, 0.7.1, 0.8.0, 0.8.1, 0.8.2, 0.9.0, 0.9.1, 0.10.0, 0.10.1, 0.10.2, 0.10.3, 0.10.4, 0.10.5, 0.11.0, 0.12.0, 0.13.0, 0.14.0, 0.15.0, 0.16.0, 0.17.0, 0.17.1, 0.18.0, 0.19.0, 0.19.1, 0.19.2, 0.20.0, 0.21.0, 0.21.1)
ERROR: No matching distribution found for python-dotenv==1.0.0
|
Smoke test added! |
|
Seems to have a lint error: https://github.com/skypilot-org/skypilot/actions/runs/5650849459/job/15308179724?pr=2296. |
Fixes #2295
A new argument is added
--env-file. This reads from a dotenv file to read multiple environment variables. These are merged with any values passed via--envarguments with--envtaking precedence in any conflicts.No unittests seem to exist for this. Manually ran the command:
with a local config that depends on two env variables within
.env.Tested (run the relevant ones):
bash format.shpytest tests/test_smoke.pypytest tests/test_smoke.py::test_fill_in_the_namebash tests/backward_comaptibility_tests.sh