[RPD-250] Move _show_terraform_outputs() into provision (#188)

* updates core and azure_runner

* adds test

* updated for comments

* updates test fixtures

src/matcha_ml/core/core.py

@@ -3,8 +3,15 @@
 from typing import Optional
 
 from matcha_ml.cli._validation import get_command_validation
-from matcha_ml.cli.ui.print_messages import print_status
-from matcha_ml.cli.ui.status_message_builders import build_warning_status
+from matcha_ml.cli.ui.print_messages import (
+    print_json,
+    print_status,
+)
+from matcha_ml.cli.ui.resource_message_builders import (
+    dict_to_json,
+    hide_sensitive_in_output,
+)
+from matcha_ml.cli.ui.status_message_builders import build_status, build_warning_status
 from matcha_ml.config import MatchaConfigService
 from matcha_ml.core._validation import is_valid_prefix, is_valid_region
 from matcha_ml.errors import MatchaError, MatchaInputError
@@ -35,6 +42,18 @@ def infer_zenml_version() -> str:
     return version
 
 
+def _show_terraform_outputs(matcha_state: MatchaState) -> None:
+    """Print the formatted Terraform outputs.
+
+    Args:
+        matcha_state (MatchaState): Terraform outputs in a MatchaState format.
+    """
+    print_status(build_status("Here are the endpoints for what's been provisioned"))
+    resources_dict = hide_sensitive_in_output(matcha_state.to_dict())
+    resources_json = dict_to_json(resources_dict)
+    print_json(resources_json)
+
+
 @track(event_name=AnalyticsEvent.GET)
 def get(
     resource_name: Optional[str],
@@ -260,8 +279,9 @@ def provision(
         )
         azure_template.build_template(config, template, destination, verbose)
 
-        template_runner.provision()
+        matcha_state_service = template_runner.provision()
 
-        matcha_state_service = MatchaStateService()
+        if verbose:
+            _show_terraform_outputs(matcha_state_service._state)
 
         return matcha_state_service.fetch_resources_from_state_file()

src/matcha_ml/runners/azure_runner.py

@@ -2,19 +2,8 @@
 import os
 import shutil
 
-from matcha_ml.cli.ui.print_messages import (
-    print_json,
-    print_status,
-)
-from matcha_ml.cli.ui.resource_message_builders import (
-    dict_to_json,
-    hide_sensitive_in_output,
-)
-from matcha_ml.cli.ui.status_message_builders import (
-    build_status,
-)
 from matcha_ml.runners.base_runner import BaseRunner
-from matcha_ml.state.matcha_state import MatchaState, MatchaStateService
+from matcha_ml.state.matcha_state import MatchaStateService
 
 
 class AzureRunner(BaseRunner):
@@ -24,34 +13,26 @@ def __init__(self) -> None:
         """Initialize AzureRunner class."""
         super().__init__()
 
-    def _show_terraform_outputs(self, matcha_state: MatchaState) -> None:
-        """Print the formatted Terraform outputs.
-
-        Args:
-            matcha_state (MatchaState): Terraform outputs in a MatchaState format.
-        """
-        print_status(build_status("Here are the endpoints for what's been provisioned"))
-        resources_dict = hide_sensitive_in_output(matcha_state.to_dict())
-        resources_json = dict_to_json(resources_dict)
-        print_json(resources_json)
-
     def remove_matcha_dir(self) -> None:
         """Removes the project's .matcha directory"."""
         project_directory = os.getcwd()
         target = os.path.join(project_directory, ".matcha")
         if os.path.exists(target):
             shutil.rmtree(target)
 
-    def provision(self) -> None:
-        """Provision resources required for the deployment."""
+    def provision(self) -> MatchaStateService:
+        """Provision resources required for the deployment.
+
+        Returns:
+            (MatchaStateService): a MatchaStateService instance initialized with Terraform output
+        """
         self._check_terraform_installation()
         self._validate_terraform_config()
         self._validate_kubeconfig(base_path=".kube/config")
         self._initialize_terraform(msg="Matcha")
         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)
+        return MatchaStateService(terraform_output=tf_output)
 
     def deprovision(self) -> None:
         """Destroy the provisioned resources."""

src/matcha_ml/runners/base_runner.py

@@ -18,6 +18,7 @@
     TerraformConfig,
     TerraformService,
 )
+from matcha_ml.state.matcha_state import MatchaStateService
 
 SPINNER = "dots"
 
@@ -182,7 +183,7 @@ def _destroy_terraform(self, msg: str = "") -> None:
             if tf_result.return_code != 0:
                 raise MatchaTerraformError(tf_error=tf_result.std_err)
 
-    def provision(self) -> Optional[Tuple[str, str, str]]:
+    def provision(self) -> MatchaStateService:
         """Provision resources required for the deployment."""
         raise NotImplementedError
 

tests/test_cli/conftest.py

@@ -29,15 +29,12 @@ def mocked_resource_template_runner() -> AzureRunner:
     ) as initialize, patch(
         f"{INTERNAL_FUNCTION_STUBS[0]}._apply_terraform"
     ) as apply, patch(
-        f"{INTERNAL_FUNCTION_STUBS[0]}._show_terraform_outputs"
-    ) as show, patch(
         f"{INTERNAL_FUNCTION_STUBS[0]}._check_terraform_installation"
     ) as check_tf_install, patch(
         f"{INTERNAL_FUNCTION_STUBS[0]}._validate_terraform_config"
     ) as validate_tf_config:
         initialize.return_value = None
         apply.return_value = None
-        show.return_value = None
         check_tf_install.return_value = None
         validate_tf_config.return_value = None
 

tests/test_core/conftest.py

@@ -54,15 +54,12 @@ def mocked_resource_template_runner() -> AzureRunner:
     ) as initialize, patch(
         f"{INTERNAL_FUNCTION_STUBS[0]}._apply_terraform"
     ) as apply, patch(
-        f"{INTERNAL_FUNCTION_STUBS[0]}._show_terraform_outputs"
-    ) as show, patch(
         f"{INTERNAL_FUNCTION_STUBS[0]}._check_terraform_installation"
     ) as check_tf_install, patch(
         f"{INTERNAL_FUNCTION_STUBS[0]}._validate_terraform_config"
     ) as validate_tf_config:
         initialize.return_value = None
         apply.return_value = None
-        show.return_value = None
         check_tf_install.return_value = None
         validate_tf_config.return_value = None
 

tests/test_core/test_core_provision.py

@@ -6,13 +6,18 @@
 from pathlib import Path
 from typing import Dict, Iterator, Union
 from unittest import mock
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 
 import pytest
+from _pytest.capture import SysCapture
 
+from matcha_ml.cli.ui.resource_message_builders import (
+    dict_to_json,
+    hide_sensitive_in_output,
+)
 from matcha_ml.core import provision
 from matcha_ml.core._validation import LONGEST_RESOURCE_NAME, MAXIMUM_RESOURCE_NAME_LEN
-from matcha_ml.core.core import infer_zenml_version
+from matcha_ml.core.core import _show_terraform_outputs, infer_zenml_version
 from matcha_ml.errors import MatchaError, MatchaInputError
 from matcha_ml.services.global_parameters_service import GlobalParameters
 from matcha_ml.state.matcha_state import (
@@ -341,3 +346,51 @@ def test_stale_remote_state_file_is_removed(matcha_testing_directory: str):
 def test_version_inference_latest():
     """Test checking when zenml isn't installed, the latest version is returned."""
     assert infer_zenml_version() == "latest"
+
+
+def test_show_terraform_outputs(state_file_as_object, capsys: SysCapture):
+    """Tests the _show_terraform_outputs function makes the relevant function calls and outputs the desired string.
+
+    Args:
+        state_file_as_object (MatchaState): a mocked MatchaState object for use in testing
+        capsys (SysCapture): fixture to capture stdout and stderr
+    """
+    expected_output = """Here are the endpoints for what's been provisioned"""
+    expected_cloud = """"cloud": {
+    "flavor": "azure",
+    "resource-group-name": "test_resources"
+  }"""
+    expected_container_registry = """"container-registry": {
+    "flavor": "azure",
+    "registry-name": "azure_registry_name",
+    "registry-url": "azure_container_registry"
+  }"""
+    expected_pipeline = """"pipeline": {
+    "flavor": "zenml",
+    "connection-string": "********",
+    "server-password": "********",
+    "server-url": "zen_server_url"
+  }"""
+    expected_experiment_tracker = """"experiment-tracker": {
+    "flavor": "mlflow",
+    "tracking-url": "mlflow_test_url"
+  }"""
+
+    with patch(
+        "matcha_ml.core.core.dict_to_json", side_effect=dict_to_json
+    ) as mocked_dict_to_json, patch(
+        "matcha_ml.core.core.hide_sensitive_in_output",
+        side_effect=hide_sensitive_in_output,
+    ) as mocked_hide_sensitive_output:
+        _show_terraform_outputs(state_file_as_object)
+
+        mocked_hide_sensitive_output.assert_called()
+        mocked_dict_to_json.assert_called_once()
+
+        captured = capsys.readouterr()
+
+        assert expected_output in captured.out
+        assert expected_cloud in captured.out
+        assert expected_container_registry in captured.out
+        assert expected_pipeline in captured.out
+        assert expected_experiment_tracker in captured.out

tests/test_runners/test_azure_runner.py

@@ -7,11 +7,9 @@
 from unittest.mock import MagicMock
 
 import pytest
-from _pytest.capture import SysCapture
 
 from matcha_ml.runners import AzureRunner
 from matcha_ml.state.matcha_state import (
-    MatchaState,
     MatchaStateService,
 )
 
@@ -147,7 +145,6 @@ def test_write_outputs_state(
     template_runner._validate_terraform_config = MagicMock()
     template_runner._initialize_terraform = MagicMock()
     template_runner._apply_terraform = MagicMock()
-    template_runner._show_terraform_outputs = MagicMock()
     template_runner.tfs.terraform_client.output = MagicMock(wraps=mock_output)
 
     with does_not_raise():
@@ -158,31 +155,6 @@ def test_write_outputs_state(
             assert json.load(f) == expected_outputs_show_sensitive
 
 
-def test_show_terraform_outputs(
-    template_runner: AzureRunner,
-    capsys: SysCapture,
-    expected_outputs_hide_sensitive: dict,
-    state_file_as_object: MatchaState,
-    matcha_testing_directory: str,
-):
-    """Test service shows the correct terraform output.
-
-    Args:
-        template_runner (AzureRunner): a AzureRunner object instance
-        capsys (SysCapture): fixture to capture stdout and stderr
-        expected_outputs_hide_sensitive (dict): expected output from terraform
-        state_file_as_object (MatchaState): mock MatchaState dataclass object
-        matcha_testing_directory (str): Testing directory
-    """
-    os.chdir(matcha_testing_directory)
-    with does_not_raise():
-        template_runner._show_terraform_outputs(state_file_as_object)
-        captured = capsys.readouterr()
-
-        for output in expected_outputs_hide_sensitive:
-            assert output in captured.out
-
-
 def test_remove_matcha_dir(matcha_testing_directory: str, template_runner: AzureRunner):
     """Tests service can remove the .matcha directory when required.
 
@@ -211,7 +183,6 @@ def test_provision(matcha_testing_directory: str, template_runner: AzureRunner):
     template_runner._validate_terraform_config = MagicMock()
     template_runner._initialize_terraform = MagicMock()
     template_runner._apply_terraform = MagicMock()
-    template_runner._show_terraform_outputs = MagicMock()
 
     os.makedirs(os.path.join(matcha_testing_directory, ".matcha", "infrastructure"))