Merge pull request #889 from simvue-io/wk9874/885-offline-fixes

Fix offline mode bugs

Author: kzscisoft

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/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/sender.py

@@ -217,7 +217,7 @@ def sender(
     id_mapping
         mapping of local ID to server ID
     """
-    _user_config: SimvueConfiguration = SimvueConfiguration.fetch()
+    _user_config: SimvueConfiguration = SimvueConfiguration.fetch(mode="online")
     cache_dir = cache_dir or _user_config.offline.cache
 
     cache_dir.joinpath("server_ids").mkdir(parents=True, exist_ok=True)

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

@@ -1153,6 +1153,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
@@ -1439,7 +1527,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]