rephrase confusing message on monitor creation (#67)

* rephrase confusing message on monitor creation
* fix tests

tests/helpers/test_monitor_helpers.py

@@ -128,16 +128,6 @@ def test_get_monitor_config(self) -> None:
         assert isinstance(monitor_config, Dict)
         for key in monitor_config.keys():
             assert key in ['orgId', 'datasetId', 'granularity', 'metadata', 'allowPartialTargetBatches', 'analyzers', 'monitors']
-
-    def test_get_monitor_config_not_existing_dataset_id(self, caplog) -> None:
-        with caplog.at_level("WARNING"):
-            monitor_config = get_monitor_config(
-                org_id=ORG_ID, 
-                dataset_id = "fake-dataset-id", 
-            )
-
-            assert monitor_config is None
-            assert "Could not find a monitor config for fake-dataset-id" in caplog.text
 
     def test_get_monitor(self) -> None:
         monitor = get_monitor(
@@ -153,25 +143,21 @@ def test_get_monitor(self) -> None:
             assert key in ['id', 'analyzerIds', 'schedule', 'mode', 'disabled', 'actions', 'metadata']
 
 
-    def test_get_monitor_with_wrong_configs(self, caplog) -> None:
-        with caplog.at_level("WARNING"):
-            monitor = get_monitor(
-                monitor_id="fake-monitor",
-                dataset_id=DATASET_ID,
-                org_id=ORG_ID
-            )
-            assert monitor is None
-            assert f"Could not find a monitor with id fake-monitor for {DATASET_ID}." in caplog.text
-        with caplog.at_level("WARNING"):
-            monitor = get_monitor(
-                monitor_id=MONITOR_ID,
-                dataset_id="fake-dataset-id",
-                org_id=ORG_ID
-            )
-
-            assert monitor is None
-            assert f"Could not find a monitor with id {MONITOR_ID} for fake-dataset-id." in caplog.text
-
+    def test_get_monitor_with_wrong_configs(self) -> None:
+        monitor = get_monitor(
+            monitor_id="fake-monitor",
+            dataset_id=DATASET_ID,
+            org_id=ORG_ID
+        )
+        assert monitor is None
+
+        monitor = get_monitor(
+            monitor_id=MONITOR_ID,
+            dataset_id="fake-dataset-id",
+            org_id=ORG_ID
+        )
+
+        assert monitor is None    
 
     def test_get_granularity(self) -> None:
         granularity = get_model_granularity(org_id=ORG_ID, dataset_id=DATASET_ID)

whylabs_toolkit/helpers/monitor_helpers.py

@@ -14,17 +14,20 @@
 logger = logging.getLogger(__name__)
 
 
-# TODO create deactivate_monitor
-
-
 def get_monitor_config(org_id: str, dataset_id: str, config: Config = Config()) -> Any:
     api = get_monitor_api(config=config)
     try:
         monitor_config = api.get_monitor_config_v3(org_id=org_id, dataset_id=dataset_id)
+        logger.info(f"Found monitor config for {dataset_id}")
         return monitor_config
     except NotFoundException:
-        logger.warning(f"Could not find a monitor config for {dataset_id}")
+        logger.info(f"Could not find a monitor config for {dataset_id}")
         return None
+    except ForbiddenException as e:
+        logger.warning(
+            f"You don't have access to monitor config for {dataset_id}. Did you set a correct WHYLABS_API_KEY?"
+        )
+        raise e
 
 
 def get_monitor(
@@ -37,11 +40,14 @@ def get_monitor(
     try:
         monitor = api.get_monitor(org_id=org_id, dataset_id=dataset_id, monitor_id=monitor_id)
         return monitor
-    except (ForbiddenException, NotFoundException):
+    except (NotFoundException):
+        logger.info(f"Didn't find a monitor with id {monitor_id} for {dataset_id}. Creating a new one...")
+        return None
+    except ForbiddenException as e:
         logger.warning(
-            f"Could not find a monitor with id {monitor_id} for {dataset_id}." "Did you set a correct WHYLABS_API_KEY?"
+            f"You don't have access to monitor {monitor_id} for {dataset_id}. Did you set a correct WHYLABS_API_KEY?"
         )
-        return None
+        raise e
 
 
 def get_analyzer_ids(

whylabs_toolkit/monitor/manager/monitor_setup.py

@@ -7,7 +7,12 @@
 from whylabs_toolkit.monitor.models import *
 from whylabs_toolkit.monitor.models.analyzer.targets import ColumnGroups
 from whylabs_toolkit.monitor.manager.credentials import MonitorCredentials
-from whylabs_toolkit.helpers.monitor_helpers import get_analyzers, get_monitor, get_model_granularity
+from whylabs_toolkit.helpers.monitor_helpers import (
+    get_analyzers,
+    get_monitor,
+    get_model_granularity,
+    get_monitor_config,
+)
 from whylabs_toolkit.helpers.config import Config
 
 
@@ -54,20 +59,16 @@ def __init__(self, monitor_id: str, dataset_id: Optional[str] = None, config: Co
 
         self._prefill_properties()
 
-    def _check_if_monitor_exists(self) -> Any:
+    def _check_if_monitor_exists(self) -> Optional[Monitor]:
         existing_monitor = get_monitor(
             org_id=self.credentials.org_id,
             dataset_id=self.credentials.dataset_id,
             monitor_id=self.credentials.monitor_id,
             config=self._config,
         )
-        if existing_monitor:
-            existing_monitor = Monitor.parse_obj(existing_monitor)
-            logger.info(f"Got existing {self.credentials.monitor_id} from WhyLabs!")
-        else:
-            logger.info(f"Did not find a monitor with {self.credentials.monitor_id}, creating a new one.")
-            existing_monitor = None
-        return existing_monitor
+        if existing_monitor is not None:
+            return Monitor.parse_obj(existing_monitor)
+        return None
 
     def _check_if_analyzer_exists(self) -> Any:
         existing_analyzers = get_analyzers(
@@ -76,12 +77,8 @@ def _check_if_analyzer_exists(self) -> Any:
             monitor_id=self.credentials.monitor_id,
             config=self._config,
         )
-        if existing_analyzers:
-            existing_analyzer = Analyzer.parse_obj(existing_analyzers[0])  # enforcing 1:1 relationship
-
-        else:
-            existing_analyzer = None
-        return existing_analyzer
+        if existing_analyzers is not None:
+            return Analyzer.parse_obj(existing_analyzers[0])  # enforcing 1:1 relationship
 
     def _prefill_properties(self) -> None:
         if self.monitor: