diff --git a/tests/helpers/test_monitor_helpers.py b/tests/helpers/test_monitor_helpers.py index 886a403..cbccc7e 100644 --- a/tests/helpers/test_monitor_helpers.py +++ b/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) diff --git a/whylabs_toolkit/helpers/monitor_helpers.py b/whylabs_toolkit/helpers/monitor_helpers.py index 7fb0e59..7eee236 100644 --- a/whylabs_toolkit/helpers/monitor_helpers.py +++ b/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( diff --git a/whylabs_toolkit/monitor/manager/monitor_setup.py b/whylabs_toolkit/monitor/manager/monitor_setup.py index 0c55e40..f6b9ffa 100644 --- a/whylabs_toolkit/monitor/manager/monitor_setup.py +++ b/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: