From 9cdc56874dffd7db149c3d2bcd2d1b35ff9812e5 Mon Sep 17 00:00:00 2001 From: Chris Date: Thu, 27 Jul 2023 10:04:48 +0100 Subject: [PATCH 1/5] Update message on Terraform apply for clarity --- src/matcha_ml/runners/azure_runner.py | 2 +- src/matcha_ml/runners/base_runner.py | 7 +++++-- src/matcha_ml/runners/remote_state_runner.py | 2 +- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/matcha_ml/runners/azure_runner.py b/src/matcha_ml/runners/azure_runner.py index 73cd7fd9..57c64afe 100644 --- a/src/matcha_ml/runners/azure_runner.py +++ b/src/matcha_ml/runners/azure_runner.py @@ -57,7 +57,7 @@ def provision(self) -> None: self._validate_terraform_config() self._validate_kubeconfig(base_path=".kube/config") self._initialize_terraform(msg="Matcha") - self._apply_terraform() + self._apply_terraform(msg="Matcha") tf_output = self.tfs.terraform_client.output() matcha_state_service = MatchaStateService(terraform_output=tf_output) self._show_terraform_outputs(matcha_state_service._state) diff --git a/src/matcha_ml/runners/base_runner.py b/src/matcha_ml/runners/base_runner.py index 925d0f3a..c44d8e2d 100644 --- a/src/matcha_ml/runners/base_runner.py +++ b/src/matcha_ml/runners/base_runner.py @@ -131,9 +131,12 @@ def _check_matcha_directory_exists(self) -> None: ) raise typer.Exit() - def _apply_terraform(self) -> None: + def _apply_terraform(self, msg: str = "") -> None: """Run terraform apply to create resources on cloud. + Args: + msg (str) : Message to display. Default is empty string. + Raises: MatchaTerraformError: if 'terraform apply' failed. """ @@ -149,7 +152,7 @@ def _apply_terraform(self) -> None: raise MatchaTerraformError(tf_error=tf_result.std_err) print_status( build_substep_success_status( - f"{Emojis.CHECKMARK.value} Matcha resources have been provisioned!\n" + f"{Emojis.CHECKMARK.value} {msg} resources have been provisioned!\n" ) ) diff --git a/src/matcha_ml/runners/remote_state_runner.py b/src/matcha_ml/runners/remote_state_runner.py index 921ada2e..99b72dfc 100644 --- a/src/matcha_ml/runners/remote_state_runner.py +++ b/src/matcha_ml/runners/remote_state_runner.py @@ -63,7 +63,7 @@ def provision(self) -> Tuple[str, str, str]: self._validate_terraform_config() self._validate_kubeconfig(base_path=".kube/config") self._initialize_terraform(msg="Remote State") - self._apply_terraform() + self._apply_terraform(msg="Remote State") return self._get_terraform_output() def deprovision(self) -> None: From 2d2ec3c4349d5de7dba373920bb95a21b5e53532 Mon Sep 17 00:00:00 2001 From: Chris Date: Thu, 27 Jul 2023 10:11:33 +0100 Subject: [PATCH 2/5] Update test with new message --- tests/test_runners/test_base_runner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_runners/test_base_runner.py b/tests/test_runners/test_base_runner.py index c7a91a43..77d30656 100644 --- a/tests/test_runners/test_base_runner.py +++ b/tests/test_runners/test_base_runner.py @@ -119,9 +119,9 @@ def test_apply_terraform(capsys: SysCapture): """ template_runner = BaseRunner() template_runner.tfs.apply = MagicMock(return_value=TerraformResult(0, "", "")) - expected = "Matcha resources have been provisioned!" + expected = "Remote State resources have been provisioned!" - template_runner._apply_terraform() + template_runner._apply_terraform(msg="Remote State") captured = capsys.readouterr() assert expected in captured.out From 8e98e68a52b89988233d4a7a0b77c76de0849fc6 Mon Sep 17 00:00:00 2001 From: Chris <32800386+Christopher-Norman@users.noreply.github.com> Date: Thu, 27 Jul 2023 13:44:17 +0100 Subject: [PATCH 3/5] Update base_runner.py --- src/matcha_ml/runners/base_runner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/matcha_ml/runners/base_runner.py b/src/matcha_ml/runners/base_runner.py index c44d8e2d..39f21d14 100644 --- a/src/matcha_ml/runners/base_runner.py +++ b/src/matcha_ml/runners/base_runner.py @@ -131,11 +131,11 @@ def _check_matcha_directory_exists(self) -> None: ) raise typer.Exit() - def _apply_terraform(self, msg: str = "") -> None: + def _apply_terraform(self, msg: str) -> None: """Run terraform apply to create resources on cloud. Args: - msg (str) : Message to display. Default is empty string. + msg (str) : Name of the type of resource (e.g. "Remote State" or "Matcha"). Raises: MatchaTerraformError: if 'terraform apply' failed. From 40b4cb8438ce282c9eb1b31923e3744631320246 Mon Sep 17 00:00:00 2001 From: Chris Date: Thu, 27 Jul 2023 13:47:07 +0100 Subject: [PATCH 4/5] Fix test --- tests/test_runners/test_base_runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_runners/test_base_runner.py b/tests/test_runners/test_base_runner.py index 77d30656..e3f91f98 100644 --- a/tests/test_runners/test_base_runner.py +++ b/tests/test_runners/test_base_runner.py @@ -131,7 +131,7 @@ def test_apply_terraform(capsys: SysCapture): ) with pytest.raises(MatchaTerraformError) as exc_info: - template_runner._apply_terraform() + template_runner._apply_terraform("Matcha") assert ( str(exc_info.value) == "Terraform failed because of the following error: 'Apply failed'." From 18d7b48e629b8fb154ba97f62c57dc39549a1902 Mon Sep 17 00:00:00 2001 From: Chris Date: Thu, 27 Jul 2023 14:05:36 +0100 Subject: [PATCH 5/5] Revert message parameter to empty string --- src/matcha_ml/runners/base_runner.py | 18 +++++++++++++----- tests/test_runners/test_base_runner.py | 2 +- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/matcha_ml/runners/base_runner.py b/src/matcha_ml/runners/base_runner.py index 39f21d14..f6e394c0 100644 --- a/src/matcha_ml/runners/base_runner.py +++ b/src/matcha_ml/runners/base_runner.py @@ -131,7 +131,7 @@ def _check_matcha_directory_exists(self) -> None: ) raise typer.Exit() - def _apply_terraform(self, msg: str) -> None: + def _apply_terraform(self, msg: str = "") -> None: """Run terraform apply to create resources on cloud. Args: @@ -150,11 +150,19 @@ def _apply_terraform(self, msg: str) -> None: if tf_result.return_code != 0: raise MatchaTerraformError(tf_error=tf_result.std_err) - print_status( - build_substep_success_status( - f"{Emojis.CHECKMARK.value} {msg} resources have been provisioned!\n" + + if msg: + print_status( + build_substep_success_status( + f"{Emojis.CHECKMARK.value} {msg} resources have been provisioned!\n" + ) + ) + else: + print_status( + build_substep_success_status( + f"{Emojis.CHECKMARK.value} Resources have been provisioned!\n" + ) ) - ) def _destroy_terraform(self, msg: str = "") -> None: """Destroy the provisioned resources. diff --git a/tests/test_runners/test_base_runner.py b/tests/test_runners/test_base_runner.py index e3f91f98..77d30656 100644 --- a/tests/test_runners/test_base_runner.py +++ b/tests/test_runners/test_base_runner.py @@ -131,7 +131,7 @@ def test_apply_terraform(capsys: SysCapture): ) with pytest.raises(MatchaTerraformError) as exc_info: - template_runner._apply_terraform("Matcha") + template_runner._apply_terraform() assert ( str(exc_info.value) == "Terraform failed because of the following error: 'Apply failed'."