test: add unit tests for dashboard generation and error handling

Author: Sushanth Sathish Kumar

src/sagemaker/model_monitor/model_monitoring.py

@@ -1543,7 +1543,6 @@ def _check_dashboard_validity_without_checking_in_use(
         self,
         monitor_schedule_name,
         enable_cloudwatch_metrics=True,
-        enable_automatic_dashboard=False,
         dashboard_name=None,
     ):
         """Checks if the parameters are valid, without checking if dashboard name is taken
@@ -1552,44 +1551,33 @@ def _check_dashboard_validity_without_checking_in_use(
             monitor_schedule_name (str): Monitoring schedule name.
             enable_cloudwatch_metrics (bool): Whether to publish cloudwatch metrics as part of
                 the baselining or monitoring jobs.
-            enable_automatic_dashboard (bool): Whether to publish dashboard as part of
-                the monitoring job.
             dashboard_name (str): The name to use when publishing dashboard
         """
-        if (not enable_cloudwatch_metrics) and enable_automatic_dashboard:
+        if not enable_cloudwatch_metrics:
             message = (
                 "Could not create automatic dashboard. "
                 "Please set enable_cloudwatch_metrics to True."
             )
             _LOGGER.error(message)
             raise ValueError(message)
 
-        if (not enable_automatic_dashboard) and dashboard_name is not None:
+        if dashboard_name is None:
+            dashboard_name = monitor_schedule_name
+        dashboard_name_validation = bool(re.match(r"^[0-9A-Za-z\-_]{1,255}$", dashboard_name))
+        if not dashboard_name_validation:
             message = (
-                "Parameter dashboard_name was provided, but enable_automatic_dashboard "
-                "parameter was False. Dashboard will not be generated."
+                f"Dashboard name {dashboard_name} is not a valid dashboard name. "
+                "Dashboard name can be at most 255 characters long "
+                "and valid characters in dashboard names include '0-9A-Za-z-_'."
             )
-            _LOGGER.warning(message)
-
-        if enable_automatic_dashboard:
-            # verify that the provided dashboard name is not taken
-            dashboard_name = monitor_schedule_name if dashboard_name is None else dashboard_name
-
-            dashboard_name_validation = bool(re.match(r"^[0-9A-Za-z\-_]{1,255}$", dashboard_name))
-            if not dashboard_name_validation:
-                message = (
-                    f"Dashboard name {dashboard_name} is not a valid dashboard name. "
-                    "Dashboard name can be at most 255 characters long "
-                    "and valid characters in dashboard names include '0-9A-Za-z-_'."
-                )
-                _LOGGER.error(message)
-                raise ValueError(message)
+            _LOGGER.error(message)
+            raise ValueError(message)
 
     def _check_automatic_dashboard_validity(
         self,
+        cw_client,
         monitor_schedule_name,
         enable_cloudwatch_metrics=True,
-        enable_automatic_dashboard=False,
         dashboard_name=None,
     ):
         """Checks if the parameters provided to generate an automatic dashboard are valid
@@ -1598,35 +1586,30 @@ def _check_automatic_dashboard_validity(
             monitor_schedule_name (str): Monitoring schedule name.
             enable_cloudwatch_metrics (bool): Whether to publish cloudwatch metrics as part of
                 the baselining or monitoring jobs.
-            enable_automatic_dashboard (bool): Whether to publish dashboard as part of
-                the monitoring job.
             dashboard_name (str): The name to use when publishing dashboard
         """
 
         self._check_dashboard_validity_without_checking_in_use(
             monitor_schedule_name=monitor_schedule_name,
             enable_cloudwatch_metrics=enable_cloudwatch_metrics,
-            enable_automatic_dashboard=enable_automatic_dashboard,
             dashboard_name=dashboard_name,
         )
-        cw_client = self.sagemaker_session.boto_session.client("cloudwatch")
 
-        if enable_automatic_dashboard:
-            try:
-                # try to access the dashboard to see if it exists already
-                cw_client.get_dashboard(DashboardName=dashboard_name)
-                message = (
-                    f"Dashboard name {dashboard_name} is already in use. "
-                    "Please provide a different dashboard name, or delete the already "
-                    "existing dashboard."
-                )
-                _LOGGER.error(message)
-                raise ValueError(message)
-            except Exception as e:
-                _LOGGER.log(f"Correctly received error {e}.")
-                # in this case, the dashboard name is not in use
-                # and we are free to write to it without overwriting any
-                # customer data.
+        # flag to check if dashboard with name dashboard_name exists already
+        dashboard_exists = True
+        try:
+            cw_client.get_dashboard(DashboardName=dashboard_name)
+        except Exception as e:
+            dashboard_exists = False
+
+        if dashboard_exists:
+            message = (
+                f"Dashboard name {dashboard_name} is already in use. "
+                "Please provide a different dashboard name, or delete the already "
+                "existing dashboard."
+            )
+            _LOGGER.error(message)
+            raise ValueError(message)
 
     def _create_monitoring_schedule_from_job_definition(
         self,
@@ -2113,10 +2096,11 @@ def create_monitoring_schedule(
         )
 
         if enable_automatic_dashboard:
+            cw_client = self.sagemaker_session.boto_session.client("cloudwatch")
             self._check_automatic_dashboard_validity(
+                cw_client=cw_client,
                 monitor_schedule_name=monitor_schedule_name,
                 enable_cloudwatch_metrics=enable_cloudwatch_metrics,
-                enable_automatic_dashboard=enable_automatic_dashboard,
                 dashboard_name=dashboard_name,
             )
 
@@ -2214,8 +2198,8 @@ def update_monitoring_schedule(
         env=None,
         network_config=None,
         enable_cloudwatch_metrics=None,
-        # enable_automatic_dashboard=False,
-        # dashboard_name=None,
+        enable_automatic_dashboard=False,
+        dashboard_name=None,
         role=None,
         batch_transform_input=None,
         data_analysis_start_time=None,
@@ -2302,8 +2286,6 @@ def update_monitoring_schedule(
                 env=env,
                 network_config=network_config,
                 enable_cloudwatch_metrics=enable_cloudwatch_metrics,
-                # enable_automatic_dashboard=enable_automatic_dashboard,
-                # dashboard_name=dashboard_name,
                 role=role,
                 batch_transform_input=batch_transform_input,
                 data_analysis_start_time=data_analysis_start_time,
@@ -2425,8 +2407,6 @@ def _update_data_quality_monitoring_schedule(
         statistics=None,
         schedule_cron_expression=None,
         enable_cloudwatch_metrics=None,
-        # enable_automatic_dashboard=False,
-        # dashboard_name=None,
         role=None,
         instance_count=None,
         instance_type=None,
@@ -3307,10 +3287,11 @@ def create_monitoring_schedule(
         )
 
         if enable_automatic_dashboard:
+            cw_client = self.sagemaker_session.boto_session.client("cloudwatch")
             self._check_automatic_dashboard_validity(
+                cw_client=cw_client,
                 monitor_schedule_name=monitor_schedule_name,
                 enable_cloudwatch_metrics=enable_cloudwatch_metrics,
-                enable_automatic_dashboard=enable_automatic_dashboard,
                 dashboard_name=dashboard_name,
             )
 
@@ -3373,8 +3354,6 @@ def create_monitoring_schedule(
             raise
 
         if enable_automatic_dashboard:
-            if dashboard_name is None:
-                dashboard_name = monitor_schedule_name
             if isinstance(endpoint_input, EndpointInput):
                 endpoint_name = endpoint_input.endpoint_name
             else:
@@ -3402,8 +3381,6 @@ def update_monitoring_schedule(
         constraints=None,
         schedule_cron_expression=None,
         enable_cloudwatch_metrics=None,
-        # enable_automatic_dashboard=False,
-        # dashboard_name=None,
         role=None,
         instance_count=None,
         instance_type=None,