Bug Fixes July-2023

.pre-commit-config.yaml

@@ -43,3 +43,4 @@ repos:
     rev: v1.16.1
     hooks:
       - id: typos
+        args: [--config=.typos.toml]

.typos.toml

@@ -1,5 +1,5 @@
 [files]
-extend-exclude = ["*.json", "*.js", "*.ipynb", "LICENSE", "*.css"]
+extend-exclude = ["*.json", "*.js", "*.ipynb", "LICENSE", "*.css", ".gitignore"]
 
 [default.extend-identifiers]
 HashiCorp = "HashiCorp"

docs/azure-permissions.md

@@ -10,9 +10,9 @@ As a provisioning tool, Matcha interacts with Azure on your behalf, hiding away
 
 ## What permissions does Matcha require?
 
-In its current form, the following Azure permissions are required:
+Your account is required to have **either**:
 
-1. Owner
+1. Owner; _OR_
 2. A combination of: Contributor + User Access Administrator
 
 > Note: These are high level roles with a lot of privileges and we're actively working on introducing more granular permissions.

docs/getting-started.md

@@ -27,6 +27,7 @@ Next, you'll need to install a couple of things.
 
 * Python 3.8 or newer, along with Virtual Env and PIP.
 * The Azure command line tool. Instructions on installing this can be found [here](https://learn.microsoft.com/en-us/cli/azure/install-azure-cli).
+* Docker. This is used to build images locally, before running them on Azure. Instructions for installing Docker can be found [here](https://www.docker.com/). The Docker daemon will need to be running on your system.
 * Terraform. We use this to provision services inside Azure. You'll find installation instructions for your platform [here](https://developer.hashicorp.com/terraform/downloads?product_intent=terraform). We recommend version 1.4 or newer.
 
 # The movie recommender
@@ -73,7 +74,7 @@ az login
 
 When you run this command, you'll be taken to the Azure login screen in a web browser window, and you'll be asked if you want to allow the Azure CLI to access your Azure account. You'll need to grant this permission in order for Matcha to gain access to your Azure account when it provisions infrastructure.
 
-> Note: you'll need certain permissions in order for Matcha to work. If you're unsure, you can just run `matcha` and it will tell you if you're missing any permissions. For specifics around permissions, please see our explainer on [Azure Permissions](azure-permissions.md).
+> Note: you'll need certain permissions for Matcha to work. If you're unsure, you can run `matcha provision` and if your Azure account is missing the required permissions, the `provision` command will tell you. For specifics around permissions, please see our explainer on [Azure Permissions](azure-permissions.md).
 
 Next, let's provision:
 
@@ -232,4 +233,6 @@ The final thing you'll want to do is decommission the infrastructure that Matcha
 matcha destroy
 ```
 
-> Note that this command is irreversible will remove all the resources deployed by `matcha provision` including the resource group, so make sure you save any data you wish to keep before running this command.
+> Note: that this command is irreversible will remove all the resources deployed by `matcha provision` including the resource group, so make sure you save any data you wish to keep before running this command.
+> 
+> You may also notice that an additional resource has appeared in Azure called 'NetworkWatcherRG' (if it wasn't already there). This is a resource that is automatically provisioned by Azure in each region when there is in-coming traffic to a provisioned resource and isn't controlled by Matcha. More information can be found [here](https://learn.microsoft.com/en-us/azure/network-watcher/network-watcher-monitoring-overview) on how to manage or remove this resource.

docs/inside-matcha.md

@@ -33,3 +33,5 @@ The user can use `get` to request information on specific resources or propertie
 ## `destroy`
 
 Once the user has finished with their provisioned environment, `destroy` enables them to tear down the resources. It works by calling the `destroy` Terraform command via the `python-terraform` library, which interacts with the configured Terraform files in the `.matcha/` directory.
+
+> You may also notice that an additional resource has appeared in Azure called 'NetworkWatcherRG' (if it wasn't already there). This is a resource that is automatically provisioned by Azure in each region when there is in-coming traffic to a provisioned resource and isn't controlled by Matcha. More information can be found [here](https://learn.microsoft.com/en-us/azure/network-watcher/network-watcher-monitoring-overview) on how to manage or remove this resource.

docs/references.md

@@ -1,3 +1,3 @@
 # API Reference Documentation
 
-::: src.matcha_ml.core.core
+::: src.matcha_ml.core.core

pyproject.toml

@@ -155,5 +155,5 @@ module = [
 ignore_missing_imports = true
 
 [tool.ruff.pylint]
-max-branches = 13
+max-branches = 14
 max-args = 6

src/matcha_ml/cli/constants.py

@@ -20,3 +20,14 @@
     ("Azure Resource Group", "The resource group containing the provisioned resources"),
     ("Matcha State Container", "A storage container for tracking matcha state"),
 ]
+
+INFRA_FACTS = [
+    "Did you know that Matcha tea was created by accident?",
+    "The brewing temperature of the water affects the taste of Matcha",
+    "Samurai's drank Matcha before battles",
+    "Matcha is provisioning Kubernetes which orchestrates tools",
+    "Seldon Core is used for model deployment",
+    "MLflow is used as an experiment tracker",
+    "Matcha is maintained by Fuzzy Labs",
+    "Everything being provisioned is fully open source",
+]

src/matcha_ml/cli/ui/spinner.py

@@ -30,9 +30,14 @@ def __init__(self, status: str):
         )
         self.progress.add_task(description=status, total=None)
 
-    def __enter__(self) -> None:
-        """Call when a spinner object is created using a `with` statement."""
+    def __enter__(self) -> "Spinner":
+        """Call when a spinner object is created using a `with` statement.
+
+        Returns:
+            Spinner: the instance for a context manager.
+        """
         self.progress.start()
+        return self
 
     def __exit__(
         self,

src/matcha_ml/cli/ui/status_message_builders.py

@@ -1,8 +1,13 @@
 """UI status message builders."""
+import time
+from random import shuffle
 from typing import List, Optional, Tuple
 
 from rich.console import Console
 
+from matcha_ml.cli.constants import INFRA_FACTS
+from matcha_ml.cli.ui.spinner import Spinner
+
 err_console = Console(stderr=True)
 
 
@@ -83,3 +88,20 @@ def build_warning_status(status: str) -> str:
         str: formatted message
     """
     return f"[yellow]{status}[/yellow]"
+
+
+def terraform_status_update(spinner: Spinner) -> None:
+    """Outputs some facts about the deployment and matcha tea while the terraform functions are running.
+
+    Args:
+        spinner: The rich spinner that the messages are printed above.
+    """
+    infra_facts_shuffled = list(range(len(INFRA_FACTS)))
+    shuffle(infra_facts_shuffled)
+
+    time.sleep(10)  # there should be a delay prior to spitting facts.
+
+    while True:
+        fact = INFRA_FACTS[infra_facts_shuffled.pop()]
+        spinner.progress.console.print(build_status(fact))
+        time.sleep(10)

src/matcha_ml/core/core.py

@@ -1,6 +1,7 @@
 """The core functionality for Matcha API."""
 import os
 from typing import Optional
+from warnings import warn
 
 from matcha_ml.cli._validation import get_command_validation
 from matcha_ml.cli.ui.print_messages import print_status
@@ -15,6 +16,22 @@
 from matcha_ml.templates.azure_template import AzureTemplate
 
 
+MAJOR_MINOR_ZENML_VERSION = "0.36"
+
+
+def zenml_version_is_supported() -> None:
+    """Check the zenml version of the local environment against the version matcha is expecting."""
+    try:
+        import zenml
+        if zenml.__version__[:3] != MAJOR_MINOR_ZENML_VERSION:
+            warn(
+                f"Matcha expects ZenML version {MAJOR_MINOR_ZENML_VERSION}.x, but you have version {zenml.__version__}."
+            )
+    except:
+        warn(f"No local installation of ZenMl found. Defaulting to version {MAJOR_MINOR_ZENML_VERSION} for remote "
+             f"resources.")
+
+
 @track(event_name=AnalyticsEvent.GET)
 def get(
     resource_name: Optional[str],
@@ -117,7 +134,9 @@ def destroy() -> None:
         )
 
     template_runner = AzureRunner()
-    with remote_state_manager.use_lock(), remote_state_manager.use_remote_state():
+    with remote_state_manager.use_lock(
+        destroy=True
+    ), remote_state_manager.use_remote_state(destroy=True):
         template_runner.deprovision()
         remote_state_manager.deprovision_remote_state()
 
@@ -184,6 +203,7 @@ def provision(
         MatchaError: If prefix is not valid.
         MatchaError: If region is not valid.
     """
+    zenml_version_is_supported()
     remote_state_manager = RemoteStateManager()
     template_runner = AzureRunner()
 

src/matcha_ml/errors.py

@@ -81,7 +81,5 @@ def __init__(self, tf_error: str, *args: Any, **kwargs: Any):
             *args: args
             **kwargs: kwargs
         """
-        message = (
-            f"Terraform failed because of the following error: '{tf_error}'."
-        )
+        message = f"Terraform failed because of the following error: '{tf_error}'."
         super().__init__(message, *args, **kwargs)

src/matcha_ml/infrastructure/remote_state_storage/output.tf

@@ -22,3 +22,8 @@ output "cloud_azure_location"{
   description = "The Azure location in which the resources are provisioned" 
   value = var.location
 }
+
+output "cloud_azure_resource_group_name" {
+  description = "Name of the Azure resource group"
+  value = module.resource_group.name
+}

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)

src/matcha_ml/runners/base_runner.py

@@ -1,5 +1,6 @@
 """Run terraform templates to provision and deprovision resources."""
 import os
+from multiprocessing.pool import ThreadPool
 from typing import Optional, Tuple
 
 import typer
@@ -10,9 +11,13 @@
 from matcha_ml.cli.ui.status_message_builders import (
     build_status,
     build_substep_success_status,
+    terraform_status_update,
 )
 from matcha_ml.errors import MatchaTerraformError
-from matcha_ml.services.terraform_service import TerraformConfig, TerraformService
+from matcha_ml.services.terraform_service import (
+    TerraformConfig,
+    TerraformService,
+)
 
 SPINNER = "dots"
 
@@ -95,11 +100,11 @@ def _initialize_terraform(self, msg: str = "", destroy: bool = False) -> None:
             )
 
             with Spinner("Initializing"):
-                ret_code, _, err = self.tfs.init()
+                tf_result = self.tfs.init()
 
-                if ret_code != 0:
+                if tf_result.return_code != 0:
                     print_error("The command 'terraform init' failed.")
-                    raise MatchaTerraformError(tf_error=err)
+                    raise MatchaTerraformError(tf_error=tf_result.std_err)
 
             print_status(
                 build_substep_success_status(
@@ -126,22 +131,38 @@ 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) : Name of the type of resource (e.g. "Remote State" or "Matcha").
+
         Raises:
             MatchaTerraformError: if 'terraform apply' failed.
         """
-        with Spinner("Applying"):
-            ret_code, _, err = self.tfs.apply()
+        with Spinner("Applying") as spinner:
+            pool = ThreadPool(processes=1)
+            _ = pool.apply_async(terraform_status_update, (spinner,))
 
-            if ret_code != 0:
-                raise MatchaTerraformError(tf_error=err)
-        print_status(
-            build_substep_success_status(
-                f"{Emojis.CHECKMARK.value} Matcha resources have been provisioned!\n"
+            tf_result = self.tfs.apply()
+
+            pool.terminate()
+
+            if tf_result.return_code != 0:
+                raise MatchaTerraformError(tf_error=tf_result.std_err)
+
+        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.
@@ -156,10 +177,10 @@ def _destroy_terraform(self, msg: str = "") -> None:
         )
         print()
         with Spinner("Destroying"):
-            ret_code, _, err = self.tfs.destroy()
+            tf_result = self.tfs.destroy()
 
-            if ret_code != 0:
-                raise MatchaTerraformError(tf_error=err)
+            if tf_result.return_code != 0:
+                raise MatchaTerraformError(tf_error=tf_result.std_err)
 
     def provision(self) -> Optional[Tuple[str, str, str]]:
         """Provision resources required for the deployment."""

src/matcha_ml/runners/remote_state_runner.py

@@ -38,9 +38,7 @@ def _get_terraform_output(self) -> Tuple[str, str, str]:
 
         prefix = "remote_state_storage"
         account_name = tf_outputs[f"{prefix}_account_name"]["value"]
-        resource_group_name = tf_outputs[f"{prefix}_resource_group_name"][
-            "value"
-        ]
+        resource_group_name = tf_outputs[f"{prefix}_resource_group_name"]["value"]
         container_name = tf_outputs[f"{prefix}_container_name"]["value"]
 
         return account_name, container_name, resource_group_name
@@ -65,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:

src/matcha_ml/services/analytics_service.py

@@ -3,6 +3,7 @@
 This approach to collecting usage data was inspired by ZenML; source: https://github.com/zenml-io/zenml/blob/main/src/zenml/utils/analytics_utils.py
 """
 import functools
+import logging
 from enum import Enum
 from time import perf_counter
 from typing import Any, Callable, Optional, Tuple
@@ -15,7 +16,10 @@
 from matcha_ml.services.global_parameters_service import GlobalParameters
 from matcha_ml.state import MatchaState, MatchaStateService
 
-analytics.write_key = "qwBKAvY6MEUvv5XIs4rE07ohf5neT3sx"
+WRITE_KEY = "qwBKAvY6MEUvv5XIs4rE07ohf5neT3sx"
+
+# Suppress Segment warnings
+logging.getLogger("segment").setLevel(logging.FATAL)
 
 
 class AnalyticsEvent(str, Enum):
@@ -27,12 +31,14 @@ class AnalyticsEvent(str, Enum):
 
 
 def execute_analytics_event(
-    func: Callable, *args, **kwargs
+    func: Callable[..., Any], *args: Any, **kwargs: Any
 ) -> Tuple[Optional[MatchaState], Any]:
     """Exists to Temporarily fix misleading error messages coming from track decorator.
 
     Args:
         func (Callable): The function decorated by track.
+        *args (Any): arguments passed to the function.
+        **kwargs (Any): additional key word arguments passed to the function.
 
     Returns:
         The result of the call to func, the error code.
@@ -57,6 +63,9 @@ def decorator(func: Callable[..., Any]) -> Callable[..., Any]:
 
         Args:
             func (Callable[..., Any]): The function that is being decorated
+
+        Returns:
+            Callable[..., Any]: The function that is being decorated
         """
 
         @functools.wraps(func)
@@ -110,11 +119,13 @@ def inner(*args: Any, **kwargs: Any) -> Any:
                     result, error_code = execute_analytics_event(func, *args, **kwargs)
                     te = perf_counter()
 
-                analytics.track(
+                client = analytics.Client(WRITE_KEY, max_retries=1, debug=False)
+
+                client.track(
                     global_params.user_id,
                     event_name.value,
                     {
-                        "time_taken": te - ts,
+                        "time_taken": float(te) - float(ts),  # type: ignore
                         "error_type": f"{error_code.__class__}.{error_code.__class__.__name__}",
                         "command_succeeded": error_code is None,
                         "matcha_state_uuid": matcha_state_uuid,