Repository: AgnostiqHQ/covalent
Pull request #1921 (2024-01-29 23:36:08)

Allow non-string infra defaults for plugins
Fix for https://github.com/AgnostiqHQ/covalent/issues/1920

- [x] I have added the tests to cover my changes.
- [x] I have updated the documentation and CHANGELOG accordingly.
- [x] I have read the CONTRIBUTING document.


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1921?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) Report
Attention: `13 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`8d22e7d`)](https://app.codecov.io/gh/AgnostiqHQ/covalent/commit/8d22e7d1b3c6078fb7fe1d63939ad064f7dcc04c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) 84.52% compared to head [(`b35a110`)](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1921?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) 85.00%.

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1921/graphs/tree.svg?width=650&height=150&src=pr&token=YGHCB3DE4P&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ)](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1921?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ)

```diff
@@             Coverage Diff             @@
##           develop    #1921      +/-   ##
===========================================
+ Coverage    84.52%   85.00%   +0.48%     
===========================================
  Files          295      181     -114     
  Lines        14488    11291    -3197     
  Branches       195        0     -195     
===========================================
- Hits         12246     9598    -2648     
+ Misses        2108     1693     -415     
+ Partials       134        0     -134     
```

| [Flag](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1921/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [Dispatcher](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1921/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) | `92.53% <ø> (ø)` | | Carriedforward from [8d22e7d](https://app.codecov.io/gh/AgnostiqHQ/covalent/commit/8d22e7d1b3c6078fb7fe1d63939ad064f7dcc04c?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) |
| [Functional_Tests](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1921/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) | `?` | | |
| [SDK](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1921/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) | `79.34% <13.33%> (-0.15%)` | :arrow_down: | |
| [UI_Backend](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1921/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) | `?` | | |
| [UI_Frontend](https://app.codecov.io/gh/AgnostiqHQ/covalent/pull/1921/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) | `?` | | |

*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AgnostiqHQ) to find out more.

</details>

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 47435213..17cf7a24 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 974e2f61..eb63bb60 100644
--- a/covalent/cloud_resource_manager/core.py
+++ b/covalent/cloud_resource_manager/core.py
@@ -24,7 +24,7 @@ import sys
 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 @@ class CloudResourceManager:
                 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 @@ class CloudResourceManager:
 
         # 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)


Test patch:
diff --git a/tests/covalent_tests/cloud_resource_manager/core_test.py b/tests/covalent_tests/cloud_resource_manager/core_test.py
index 7fdca350..b9b0f2ab 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.