Merge branch 'dev' into kzscisoft/sender-refactor

Author: kzscisoft

simvue/api/objects/alert/base.py

@@ -31,6 +31,7 @@ def __init__(self, identifier: str | None = None, **kwargs) -> None:
         """Retrieve an alert from the Simvue server by identifier"""
         self._label = "alert"
         super().__init__(identifier=identifier, **kwargs)
+        self._local_only_args = ["frequency", "pattern", "aggregation"]
 
     def compare(self, other: "AlertBase") -> bool:
         """Compare this alert to another"""

simvue/api/objects/alert/metrics.py

@@ -30,6 +30,12 @@ def __init__(self, identifier: str | None = None, **kwargs) -> None:
         """Connect to a local or remote threshold alert by identifier"""
         self.alert = MetricThresholdAlertDefinition(self)
         super().__init__(identifier, **kwargs)
+        self._local_only_args += [
+            "rule",
+            "window",
+            "metric",
+            "threshold",
+        ]
 
     @classmethod
     def get(
@@ -117,6 +123,13 @@ def __init__(self, identifier: str | None = None, **kwargs) -> None:
         """Connect to a local or remote threshold alert by identifier"""
         self.alert = MetricRangeAlertDefinition(self)
         super().__init__(identifier, **kwargs)
+        self._local_only_args += [
+            "rule",
+            "window",
+            "metric",
+            "range_low",
+            "range_high",
+        ]
 
     def compare(self, other: "MetricsRangeAlert") -> bool:
         """Compare two MetricRangeAlerts"""

simvue/api/objects/artifact/base.py

@@ -54,6 +54,7 @@ def __init__(
         self._label = "artifact"
         self._endpoint = f"{self._label}s"
         super().__init__(identifier=identifier, _read_only=_read_only, **kwargs)
+        self._local_only_args += ["storage", "file_path", "runs"]
 
         # If the artifact is an online instance, need a place to store the response
         # from the initial creation

simvue/api/objects/artifact/file.py

@@ -80,7 +80,9 @@ def new(
         else:
             file_path = pathlib.Path(file_path)
             if snapshot:
-                _user_config = SimvueConfiguration.fetch()
+                _user_config = SimvueConfiguration.fetch(
+                    mode="offline" if offline else "online"
+                )
 
                 _local_staging_dir: pathlib.Path = _user_config.offline.cache.joinpath(
                     "artifacts"

simvue/api/objects/base.py

@@ -196,6 +196,12 @@ def __init__(
         self._read_only: bool = _read_only
         self._is_set: bool = False
         self._endpoint: str = getattr(self, "_endpoint", f"{self._label}s")
+
+        # For simvue object initialisation, unlike the server there is no nested
+        # arguments, however this means that there are extra keys during post which
+        # need removing, this attribute handles that and should be set in subclasses.
+        self._local_only_args: list[str] = []
+
         self._identifier: str | None = (
             identifier if identifier is not None else f"offline_{uuid.uuid1()}"
         )
@@ -626,6 +632,10 @@ def _post_single(
         if not is_json:
             kwargs = msgpack.packb(data or kwargs, use_bin_type=True)
 
+        # Remove any extra keys
+        for key in self._local_only_args:
+            _ = (data or kwargs).pop(key, None)
+
         _response = sv_post(
             url=f"{self._base_url}",
             headers=self._headers | {"Content-Type": "application/msgpack"},
@@ -663,6 +673,11 @@ def _post_single(
     def _put(self, **kwargs) -> dict[str, typing.Any]:
         if not self.url:
             raise RuntimeError(f"Identifier for instance of {self._label} Unknown")
+
+        # Remove any extra keys
+        for key in self._local_only_args:
+            _ = kwargs.pop(key, None)
+
         _response = sv_put(
             url=f"{self.url}", headers=self._headers, data=kwargs, is_json=True
         )

simvue/api/objects/run.py

@@ -696,19 +696,9 @@ def on_reconnect(self, id_mapping: dict[str, str]) -> None:
         id_mapping: dict[str, str]
             A mapping from offline identifier to online identifier.
         """
-        online_alert_ids: list[str] = []
-        for id in self._staging.get("alerts", []):
-            try:
-                online_alert_ids.append(id_mapping[id])
-            except KeyError:
-                raise KeyError(
-                    "Could not find alert ID in offline to online ID mapping."
-                )
-        # If run is offline, no alerts have been added yet, so add all alerts:
-        if self._identifier is not None and self._identifier.startswith("offline"):
-            self._staging["alerts"] = online_alert_ids
-        # Otherwise, only add alerts which have not yet been added
-        else:
-            self._staging["alerts"] = [
-                id for id in online_alert_ids if id not in list(self.alerts)
-            ]
+        online_alert_ids: list[str] = list(
+            set(id_mapping.get(_id) for _id in self._staging.get("alerts", []))
+        )
+        if not all(online_alert_ids):
+            raise KeyError("Could not find alert ID in offline to online ID mapping.")
+        self._staging["alerts"] = online_alert_ids

simvue/config/user.py

@@ -151,9 +151,9 @@ def check_valid_server(cls, values: "SimvueConfiguration") -> "SimvueConfigurati
     @sv_util.prettify_pydantic
     def fetch(
         cls,
+        mode: typing.Literal["offline", "online", "disabled"],
         server_url: str | None = None,
         server_token: str | None = None,
-        mode: typing.Literal["offline", "online", "disabled"] | None = None,
     ) -> "SimvueConfiguration":
         """Retrieve the Simvue configuration from this project
 

simvue/run.py

@@ -761,7 +761,6 @@ def init(
         self._sv_obj.alerts = []
         self._sv_obj.created = time.time()
         self._sv_obj.notifications = notification
-        self._sv_obj._staging["folder_id"] = self._folder.id
 
         if self._status == "running":
             self._sv_obj.system = get_system()

tests/functional/test_config.py

@@ -101,15 +101,16 @@ def _mocked_find(file_names: list[str], *_, ppt_file=_ppt_file, conf_file=_confi
 
         if not use_file and not use_env and not use_args:
             with pytest.raises(RuntimeError):
-                simvue.config.user.SimvueConfiguration.fetch()
+                simvue.config.user.SimvueConfiguration.fetch(mode="online")
             return
         elif use_args:
             _config = simvue.config.user.SimvueConfiguration.fetch(
                 server_url=_arg_url,
-                server_token=_arg_token
+                server_token=_arg_token,
+                mode="online"
             )
         else:
-            _config = simvue.config.user.SimvueConfiguration.fetch()
+            _config = simvue.config.user.SimvueConfiguration.fetch(mode="online")
 
         if use_file and use_file != "pyproject.toml":
             assert _config.config_file() == _config_file

tests/functional/test_run_class.py

@@ -1167,6 +1167,94 @@ def test_add_alerts() -> None:
         )
     for _id in _expected_alerts:
         client.delete_alert(_id)
+        
+@pytest.mark.run
+@pytest.mark.offline
+def test_add_alerts_offline(monkeypatch) -> None:
+    _uuid = f"{uuid.uuid4()}".split("-")[0]
+    
+    temp_d = tempfile.TemporaryDirectory()
+    monkeypatch.setenv("SIMVUE_OFFLINE_DIRECTORY", temp_d.name)
+
+    run = sv_run.Run(mode="offline")
+    run.init(
+        name="test_add_alerts_offline",
+        folder=f"/simvue_unit_testing/{_uuid}",
+        retention_period=os.environ.get("SIMVUE_TESTING_RETENTION_PERIOD", "2 mins"),
+        tags=[platform.system(), "test_add_alerts"],
+        visibility="tenant" if os.environ.get("CI") else None,
+    )
+
+    _expected_alerts = []
+
+    # Create alerts, have them attach to run automatically
+    _id = run.create_event_alert(
+        name=f"event_alert_{_uuid}",
+        pattern="test",
+    )
+    _expected_alerts.append(_id)
+
+    # Create another alert and attach to run
+    _id = run.create_metric_range_alert(
+        name=f"metric_range_alert_{_uuid}",
+        metric="test",
+        range_low=10,
+        range_high=100,
+        rule="is inside range",
+    )
+    _expected_alerts.append(_id)
+
+    # Create another alert, do not attach to run
+    _id = run.create_metric_threshold_alert(
+        name=f"metric_threshold_alert_{_uuid}",
+        metric="test",
+        threshold=10,
+        rule="is above",
+        attach_to_run=False,
+    )
+    
+    # Try redefining existing alert again
+    _id = run.create_metric_range_alert(
+        name=f"metric_range_alert_{_uuid}",
+        metric="test",
+        range_low=10,
+        range_high=100,
+        rule="is inside range",
+    )
+    
+    _id_mapping = sv_send.sender(os.environ["SIMVUE_OFFLINE_DIRECTORY"], 2, 10, throw_exceptions=True)
+    _online_run = RunObject(identifier=_id_mapping.get(run.id))
+
+    # Check that there is no duplication
+    assert sorted(_online_run.alerts) == sorted([_id_mapping.get(_id) for _id in _expected_alerts])
+
+    # Create another run without adding to run
+    _id = run.create_user_alert(name=f"user_alert_{_uuid}", attach_to_run=False)
+    _id_mapping = sv_send.sender(os.environ["SIMVUE_OFFLINE_DIRECTORY"], 2, 10, throw_exceptions=True)
+
+    # Check alert is not added
+    _online_run.refresh()
+    assert sorted(_online_run.alerts) == sorted([_id_mapping.get(_id) for _id in _expected_alerts])
+
+    # Try adding alerts with IDs, check there is no duplication
+    _expected_alerts.append(_id)
+    run.add_alerts(ids=_expected_alerts)
+    _id_mapping = sv_send.sender(os.environ["SIMVUE_OFFLINE_DIRECTORY"], 2, 10, throw_exceptions=True)
+
+    _online_run.refresh()
+    assert sorted(_online_run.alerts) == sorted([_id_mapping.get(_id) for _id in _expected_alerts])
+
+    run.close()
+
+    client = sv_cl.Client()
+    with contextlib.suppress(ObjectNotFoundError):
+        client.delete_folder(
+            f"/simvue_unit_testing/{_uuid}",
+            remove_runs=True,
+            recursive=True
+        )
+    for _id in [_id_mapping.get(_id) for _id in _expected_alerts]:
+        client.delete_alert(_id)
 
 
 @pytest.mark.run
@@ -1457,7 +1545,7 @@ def test_run_environment_metadata(environment: str, mocker: pytest_mock.MockerFi
     _target_dir = _data_dir
     if "python" in environment:
         _target_dir = _data_dir.joinpath(environment)
-    _config = SimvueConfiguration.fetch()
+    _config = SimvueConfiguration.fetch(mode="online")
 
     with sv_run.Run(server_token=_config.server.token, server_url=_config.server.url) as run:
         _uuid = f"{uuid.uuid4()}".split("-")[0]