diff --git a/CHANGELOG.md b/CHANGELOG.md index 474352138..17cf7a24d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Added CRM method to handle Python to TF value conversion (e.g. None->null, True->true, False->false). - Added `pennylane` as a requirement in tests due to the tutorials using it ### Changed diff --git a/covalent/cloud_resource_manager/core.py b/covalent/cloud_resource_manager/core.py index 974e2f61c..eb63bb604 100644 --- a/covalent/cloud_resource_manager/core.py +++ b/covalent/cloud_resource_manager/core.py @@ -24,7 +24,7 @@ from configparser import ConfigParser from pathlib import Path from types import ModuleType -from typing import Callable, Dict, List, Optional, Union +from typing import Any, Callable, Dict, List, Optional, Sequence, Union from covalent._shared_files.config import set_config from covalent.executor import _executor_manager @@ -432,8 +432,8 @@ def up(self, print_callback: Callable, dry_run: bool = True) -> None: if "default" in value: tf_vars_env_dict[f"TF_VAR_{key}"] = value["default"] - if value["default"] != "": - f.write(f'{key}="{value["default"]}"\n') + if value["default"]: + f.write(f'{key}={self._convert_to_tfvar(value["default"])}\n') # Overwrite the default values with the user passed values if self.executor_options: @@ -537,3 +537,29 @@ def status(self) -> None: # Run `terraform state list` return self._run_in_subprocess(cmd=tf_state, env_vars=self._terraform_log_env_vars) + + @staticmethod + def _convert_to_tfvar(value: Any) -> Any: + """ + Convert the value to a string that can be parsed as a terraform variable. + + Args: + value: Value to convert + + Returns: + Converted value + + """ + if value is True: + return "true" + if value is False: + return "false" + if value is None: + return "null" + if isinstance(value, str): + return f'"{value}"' + if isinstance(value, Sequence): + values = [CloudResourceManager._convert_to_tfvar(v) for v in value] + return f"[{', '.join(values)}]" + + return str(value) diff --git a/tests/covalent_tests/cloud_resource_manager/core_test.py b/tests/covalent_tests/cloud_resource_manager/core_test.py index 7fdca350d..b9b0f2ab0 100644 --- a/tests/covalent_tests/cloud_resource_manager/core_test.py +++ b/tests/covalent_tests/cloud_resource_manager/core_test.py @@ -41,6 +41,18 @@ def executor_module_path(): return "test_executor_module_path" +@pytest.fixture +def executor_infra_defaults(): + from pydantic import BaseModel + + class FakeExecutorInfraDefaults(BaseModel): + string_param: str = "fake_address_123" + number_param: int = 123 + sequence_param: tuple = (1, 2, 3) + + return FakeExecutorInfraDefaults + + @pytest.fixture def crm(mocker, executor_name, executor_module_path): mocker.patch( @@ -377,7 +389,9 @@ def test_get_tf_statefile_path(mocker, crm, executor_name): (False, {"test_key": "test_value"}), ], ) -def test_up(mocker, dry_run, executor_options, executor_name, executor_module_path): +def test_up( + mocker, dry_run, executor_options, executor_name, executor_module_path, executor_infra_defaults +): """ Unit test for CloudResourceManager.up() method """ @@ -401,10 +415,6 @@ def test_up(mocker, dry_run, executor_options, executor_name, executor_module_pa "covalent.cloud_resource_manager.core.get_executor_module", ) - mocker.patch( - "covalent.cloud_resource_manager.core.getattr", - ) - # Mocking as we are instantiating with executor options mocker.patch( "covalent.cloud_resource_manager.core.validate_options", @@ -438,6 +448,9 @@ def test_up(mocker, dry_run, executor_options, executor_name, executor_module_pa options=executor_options, ) + # Override infra defaults with dummy values. + crm.ExecutorInfraDefaults = executor_infra_defaults + with mock.patch( "covalent.cloud_resource_manager.core.open", mock.mock_open(), @@ -652,6 +665,27 @@ def test_crm_get_resource_status(mocker, crm): mock_terraform_error_validator.assert_called_once() +def test_crm_convert_to_tfvar(mocker, crm): + """ + Unit test for CloudResourceManager._convert_to_tfvar() method. + + Test conversion outcomes. + """ + _values_map = { + # Convenient test case (not valid Terraform): + (1, False, None, "covalent321"): '[1, false, null, "covalent321"]', + # Usual test cases: + True: "true", + False: "false", + None: "null", + "covalent123": '"covalent123"', # must include quotes + 16: "16", + } + + for _value, _expected in _values_map.items(): + assert crm._convert_to_tfvar(_value) == _expected + + def test_no_git(crm, mocker): """ Test for exit with status 1 if `git` is not available.