From 56ed65908128a3d994386cb8a9034269ecff625b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristian=20Zar=C4=99bski?= Date: Tue, 31 Mar 2026 11:03:58 +0100 Subject: [PATCH 1/6] Fix alert duplication and incorrectly falsified 0 values --- CHANGELOG.md | 5 ++ pyproject.toml | 85 ++++++++++++++--------------- simvue/api/objects/alert/base.py | 17 ++++++ simvue/api/objects/alert/events.py | 16 ++++-- simvue/api/objects/alert/metrics.py | 43 ++++++++++----- simvue/api/objects/alert/user.py | 10 +++- simvue/executor.py | 2 +- simvue/run.py | 41 +++++++++++++- tests/functional/test_run_class.py | 61 ++++++++++++++++++++- 9 files changed, 210 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8114245..230c9e58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Change Log +## [v2.4.1](https://github.com/simvue-io/python-api/releases/tag/v2.4.1) - 2026-03-31 + +- Fixed creation of duplicate alerts. +- Fixed case where alert specifications being `0` led to incorrect falsification. + ## [v2.4.0](https://github.com/simvue-io/python-api/releases/tag/v2.4.0) - 2026-03-23 - Added search for Simvue configuration within the parent file hierarchy of the current directory. diff --git a/pyproject.toml b/pyproject.toml index 6e3c5e88..5235beb4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,38 +1,36 @@ [project] name = "simvue" -version = "2.4.0" +version = "2.4.1" description = "Simulation tracking and monitoring" -authors = [ - {name = "Simvue Development Team", email = "info@simvue.io"} -] +authors = [{ name = "Simvue Development Team", email = "info@simvue.io" }] license = "Apache v2" requires-python = ">=3.10,<3.15" readme = "README.md" classifiers = [ - "Development Status :: 5 - Production/Stable", - "Intended Audience :: Science/Research", - "License :: OSI Approved :: Apache Software License", - "Natural Language :: English", - "Operating System :: Unix", - "Operating System :: Microsoft :: Windows", - "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.10", - "Programming Language :: Python :: 3.11", - "Programming Language :: Python :: 3.12", - "Programming Language :: Python :: 3.13", - "Programming Language :: Python :: 3.14", - "Topic :: Scientific/Engineering", - "Topic :: System :: Monitoring", - "Topic :: Utilities", - "Typing :: Typed" + "Development Status :: 5 - Production/Stable", + "Intended Audience :: Science/Research", + "License :: OSI Approved :: Apache Software License", + "Natural Language :: English", + "Operating System :: Unix", + "Operating System :: Microsoft :: Windows", + "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", + "Programming Language :: Python :: 3.14", + "Topic :: Scientific/Engineering", + "Topic :: System :: Monitoring", + "Topic :: Utilities", + "Typing :: Typed", ] keywords = [ - "tracking", - "monitoring", - "metrics", - "alerting", - "metrics-gathering" + "tracking", + "monitoring", + "metrics", + "alerting", + "metrics-gathering", ] dependencies = [ "requests (>=2.32.3,<3.0.0)", @@ -93,26 +91,25 @@ extend-exclude = ["tests", "examples", "notebooks"] [tool.pytest.ini_options] addopts = "-p no:warnings --no-cov -n 0" -testpaths = [ - "tests" -] +testpaths = ["tests"] markers = [ - "eco: tests for emission metrics", - "client: tests of Simvue client", - "dispatch: test data dispatcher", - "run: test the simvue Run class", - "utilities: test simvue utilities module", - "scenario: test scenarios", - "executor: tests of executors", - "config: tests of simvue configuration", - "api: tests of RestAPI functionality", - "unix: tests for UNIX systems only", - "metadata: tests of metadata gathering functions", - "online: tests for online functionality", - "offline: tests for offline functionality", - "local: tests of functionality which do not involve a server or writing to an offline cache file", - "object_retrieval: tests relating to retrieval of objects from the server", - "object_removal: tests relating to removal of objects from the server", + "cli: Sender CLI tests", + "eco: tests for emission metrics", + "client: tests of Simvue client", + "dispatch: test data dispatcher", + "run: test the simvue Run class", + "utilities: test simvue utilities module", + "scenario: test scenarios", + "executor: tests of executors", + "config: tests of simvue configuration", + "api: tests of RestAPI functionality", + "unix: tests for UNIX systems only", + "metadata: tests of metadata gathering functions", + "online: tests for online functionality", + "offline: tests for offline functionality", + "local: tests of functionality which do not involve a server or writing to an offline cache file", + "object_retrieval: tests relating to retrieval of objects from the server", + "object_removal: tests relating to removal of objects from the server", ] [tool.interrogate] diff --git a/simvue/api/objects/alert/base.py b/simvue/api/objects/alert/base.py index 7643a026..9dd96730 100644 --- a/simvue/api/objects/alert/base.py +++ b/simvue/api/objects/alert/base.py @@ -15,6 +15,11 @@ from simvue.api.url import URL from simvue.models import NAME_REGEX, DATETIME_FORMAT +try: + from typing import override +except ImportError: + from typing_extensions import override # noqa: UP035 + class AlertBase(SimvueObject): """Class for interfacing with Simvue alerts @@ -38,6 +43,18 @@ def __init__(self, identifier: str | None = None, **kwargs) -> None: "status", ] + @override + def __eq__(self, other: "AlertBase") -> bool: + """Check if alerts are the same.""" + return all( + [ + self.name == other.name, + self.description == other.description, + self.source == other.source, + self.notification == other.notification, + ] + ) + def compare(self, other: "AlertBase") -> bool: """Compare this alert to another""" return type(self) is type(other) and self.name == other.name diff --git a/simvue/api/objects/alert/events.py b/simvue/api/objects/alert/events.py index d9ea04c2..007a0c53 100644 --- a/simvue/api/objects/alert/events.py +++ b/simvue/api/objects/alert/events.py @@ -10,9 +10,9 @@ import pydantic try: - from typing import Self + from typing import Self, override except ImportError: - from typing_extensions import Self + from typing_extensions import Self, override from simvue.api.objects.base import write_only from .base import AlertBase, staging_check from simvue.models import NAME_REGEX @@ -109,6 +109,13 @@ def new( _alert._params = {"deduplicate": True} return _alert + @override + def __eq__(self, other: "AlertBase") -> bool: + """Compare Events Alerts.""" + if not isinstance(other, EventsAlert): + return False + return super().__eq__(other) and self.alert == other.alert + class EventAlertDefinition: """Event alert definition sub-class""" @@ -117,11 +124,8 @@ def __init__(self, alert: EventsAlert) -> None: """Initialise an alert definition with its parent alert""" self._sv_obj = alert - def compare(self, other: "EventAlertDefinition") -> bool: + def __eq__(self, other: "EventAlertDefinition") -> bool: """Compare this definition with that of another EventAlert""" - if not isinstance(other, EventAlertDefinition): - return False - return all( [ self.frequency == other.frequency, diff --git a/simvue/api/objects/alert/metrics.py b/simvue/api/objects/alert/metrics.py index 319664cd..5c8c2f92 100644 --- a/simvue/api/objects/alert/metrics.py +++ b/simvue/api/objects/alert/metrics.py @@ -22,6 +22,11 @@ Aggregate = typing.Literal["average", "sum", "at least one", "all"] Rule = typing.Literal["is above", "is below", "is inside range", "is outside range"] +try: + from typing import override +except ImportError: + from typing_extensions import override # noqa: UP035 + class MetricsThresholdAlert(AlertBase): """ @@ -134,6 +139,12 @@ def new( return _alert + @override + def __eq__(self, other: "AlertBase") -> bool: + if not isinstance(other, MetricsThresholdAlert): + return False + return super().__eq__(other) and self.alert == other.alert + class MetricsRangeAlert(AlertBase): """ @@ -169,9 +180,12 @@ def __init__(self, identifier: str | None = None, **kwargs) -> None: "range_high", ] - def compare(self, other: "MetricsRangeAlert") -> bool: + @override + def __eq__(self, other: "AlertBase") -> bool: """Compare two MetricRangeAlerts""" - return self.alert.compare(other) if super().compare(other) else False + if not isinstance(other, MetricsRangeAlert): + return False + return super().__eq__(other) and self.alert == other.alert @classmethod @pydantic.validate_call @@ -258,7 +272,7 @@ def __init__(self, alert: MetricsRangeAlert) -> None: """Initialise definition with target alert""" self._sv_obj = alert - def compare(self, other: "MetricsAlertDefinition") -> bool: + def __eq__(self, other: "MetricsAlertDefinition") -> bool: """Compare a MetricsAlertDefinition with another""" return all( [ @@ -272,7 +286,7 @@ def compare(self, other: "MetricsAlertDefinition") -> bool: @property def aggregation(self) -> Aggregate: """Retrieve the aggregation strategy for this alert""" - if not (_aggregation := self._sv_obj.get_alert().get("aggregation")): + if (_aggregation := self._sv_obj.get_alert().get("aggregation")) is None: raise RuntimeError( "Expected key 'aggregation' in alert definition retrieval" ) @@ -281,14 +295,14 @@ def aggregation(self) -> Aggregate: @property def rule(self) -> Rule: """Retrieve the rule for this alert""" - if not (_rule := self._sv_obj.get_alert().get("rule")): + if (_rule := self._sv_obj.get_alert().get("rule")) is None: raise RuntimeError("Expected key 'rule' in alert definition retrieval") return _rule @property def window(self) -> int: """Retrieve the aggregation window for this alert""" - if not (_window := self._sv_obj.get_alert().get("window")): + if (_window := self._sv_obj.get_alert().get("window")) is None: raise RuntimeError("Expected key 'window' in alert definition retrieval") return _window @@ -315,17 +329,17 @@ def frequency(self, frequency: int) -> None: class MetricThresholdAlertDefinition(MetricsAlertDefinition): """Alert definition for metric threshold alerts""" - def compare(self, other: "MetricThresholdAlertDefinition") -> bool: + def __eq__(self, other: "MetricThresholdAlertDefinition") -> bool: """Compare this MetricThresholdAlertDefinition with another""" - if not isinstance(other, MetricThresholdAlertDefinition): + if not super().__eq__(other): return False - return all([super().compare(other), self.threshold == other.threshold]) + return self.threshold == other.threshold @property def threshold(self) -> float: """Retrieve the threshold value for this alert""" - if not (threshold_l := self._sv_obj.get_alert().get("threshold")): + if (threshold_l := self._sv_obj.get_alert().get("threshold")) is None: raise RuntimeError("Expected key 'threshold' in alert definition retrieval") return threshold_l @@ -333,14 +347,13 @@ def threshold(self) -> float: class MetricRangeAlertDefinition(MetricsAlertDefinition): """Alert definition for metric range alerts""" - def compare(self, other: "MetricRangeAlertDefinition") -> bool: + def __eq__(self, other: "MetricRangeAlertDefinition") -> bool: """Compare a MetricRangeAlertDefinition with another""" - if not isinstance(other, MetricRangeAlertDefinition): + if not super().__eq__(other): return False return all( [ - super().compare(other), self.range_high == other.range_high, self.range_low == other.range_low, ] @@ -349,14 +362,14 @@ def compare(self, other: "MetricRangeAlertDefinition") -> bool: @property def range_low(self) -> float: """Retrieve the lower limit for metric range""" - if not (range_l := self._sv_obj.get_alert().get("range_low")): + if (range_l := self._sv_obj.get_alert().get("range_low")) is None: raise RuntimeError("Expected key 'range_low' in alert definition retrieval") return range_l @property def range_high(self) -> float: """Retrieve upper limit for metric range""" - if not (range_u := self._sv_obj.get_alert().get("range_high")): + if (range_u := self._sv_obj.get_alert().get("range_high")) is None: raise RuntimeError( "Expected key 'range_high' in alert definition retrieval" ) diff --git a/simvue/api/objects/alert/user.py b/simvue/api/objects/alert/user.py index 24c1b625..44786a03 100644 --- a/simvue/api/objects/alert/user.py +++ b/simvue/api/objects/alert/user.py @@ -10,9 +10,9 @@ import typing try: - from typing import Self + from typing import Self, override except ImportError: - from typing_extensions import Self + from typing_extensions import Self, override import http from simvue.api.request import get_json_from_response, put as sv_put @@ -89,6 +89,12 @@ def new( _alert._params = {"deduplicate": True} return _alert + @override + def __eq__(self, other: "AlertBase") -> bool: + if not isinstance(other, UserAlert): + return False + return super().__eq__(other) + @classmethod def get( cls, count: int | None = None, offset: int | None = None diff --git a/simvue/executor.py b/simvue/executor.py index 3d25461e..b5e7f0aa 100644 --- a/simvue/executor.py +++ b/simvue/executor.py @@ -476,7 +476,7 @@ def kill_process( if process_id is an integer, whether to kill only its children """ if isinstance(process_id, str): - if not (process := self._processes.get(process_id)): + if (process := self._processes.get(process_id)) is None: logger.error( f"Failed to terminate process '{process_id}', no such identifier." ) diff --git a/simvue/run.py b/simvue/run.py index 3ca3dae4..1b09d417 100644 --- a/simvue/run.py +++ b/simvue/run.py @@ -32,6 +32,7 @@ import click import psutil +from simvue.api.objects.alert.base import AlertBase from simvue.api.objects.alert.fetch import Alert from simvue.api.objects.folder import Folder from simvue.api.objects.grids import GridMetrics @@ -2198,6 +2199,14 @@ def add_alerts( return True + def _check_if_alert_exists(self, alert: "AlertBase") -> str | None: + """Check if an existing alert matches definition.""" + # If the alert already exists just add the existing one + for _id, _existing_alert in Alert.get(): + if _existing_alert == alert: + return _id + return None + @skip_if_failed("_aborted", "_suppress_errors", None) @pydantic.validate_call def create_metric_range_alert( @@ -2276,6 +2285,13 @@ def create_metric_range_alert( frequency=frequency or 60, offline=self._user_config.run.mode == "offline", ) + + # If the alert already exists just add the existing one + if _existing_id := self._check_if_alert_exists(_alert): + if attach_to_run: + self.add_alerts(ids=[_existing_id]) + return _existing_id + _alert.abort = trigger_abort _alert.commit() if attach_to_run: @@ -2358,6 +2374,12 @@ def create_metric_threshold_alert( offline=self._user_config.run.mode == "offline", ) + # If the alert already exists just add the existing one + if _existing_id := self._check_if_alert_exists(_alert): + if attach_to_run: + self.add_alerts(ids=[_existing_id]) + return _existing_id + _alert.abort = trigger_abort _alert.commit() if attach_to_run: @@ -2412,10 +2434,19 @@ def create_event_alert( frequency=frequency, offline=self._user_config.run.mode == "offline", ) + + # If the alert already exists just add the existing one + if _existing_id := self._check_if_alert_exists(_alert): + if attach_to_run: + self.add_alerts(ids=[_existing_id]) + return _existing_id + _alert.abort = trigger_abort _alert.commit() + if attach_to_run: self.add_alerts(ids=[_alert.id]) + return _alert.id @skip_if_failed("_aborted", "_suppress_errors", None) @@ -2428,7 +2459,7 @@ def create_user_alert( notification: typing.Literal["email", "none"] = "none", trigger_abort: bool = False, attach_to_run: bool = True, - ) -> None: + ) -> str | None: """Creates a user alert with the specified name (if it doesn't exist) and applies it to the current run. If alert already exists it will not be duplicated. @@ -2454,12 +2485,20 @@ def create_user_alert( returns the created alert ID if successful """ + _alert = UserAlert.new( name=name, notification=notification, description=description, offline=self._user_config.run.mode == "offline", ) + + # If the alert already exists just add the existing one + if _existing_id := self._check_if_alert_exists(_alert): + if attach_to_run: + self.add_alerts(ids=[_existing_id]) + return _existing_id + _alert.abort = trigger_abort _alert.commit() if attach_to_run: diff --git a/tests/functional/test_run_class.py b/tests/functional/test_run_class.py index 5f66c4d6..0b19e687 100644 --- a/tests/functional/test_run_class.py +++ b/tests/functional/test_run_class.py @@ -19,7 +19,7 @@ import random import datetime import simvue -from simvue.api.objects import Alert, Metrics +from simvue.api.objects import Alert, EventsAlert, Metrics, MetricsRangeAlert, MetricsThresholdAlert, UserAlert from simvue.api.objects.grids import GridMetrics from simvue.exception import ObjectNotFoundError, SimvueRunError from simvue.sender import Sender @@ -1593,3 +1593,62 @@ def test_run_environment_metadata(environment: str, mocker: pytest_mock.MockerFi ) run.update_metadata(env_func(_target_dir)) + +@pytest.mark.online +@pytest.mark.parametrize( + "alert_type", ("user", "events", "metric_threshold", "metric_range") +) +def test_no_alert_dupes(alert_type: Literal["user", "events", "metric_threshold", "metric_range"]) -> None: + """Test there are no duplicate alerts.""" + N_DUPLICATES: int = 2 + _uuid = f"{uuid.uuid4()}".split("-")[0] + _alert_id: str | None = None + + with simvue.Run() as run: + run.init( + name="test_no_alert_dupes", + folder=f"/simvue_unit_testing/{_uuid}", + retention_period=os.environ.get("SIMVUE_TESTING_RETENTION_PERIOD", "2 mins"), + running=False, + visibility="tenant" if os.environ.get("CI") else None, + ) + if alert_type == "user": + for _ in range(N_DUPLICATES): + _alert_id = run.create_user_alert(name="duplicate_alert_test_user") + elif alert_type == "events": + for _ in range(N_DUPLICATES): + _alert_id = run.create_event_alert( + name="duplicate_alert_test_events", + pattern="no-pattern", + + ) + elif alert_type == "metric_threshold": + for _ in range(N_DUPLICATES): + _alert_id = run.create_metric_threshold_alert( + name="duplicate_alert_test_metric_threshold", + threshold=0, + rule="is above", + metric="x" + ) + else: + for _ in range(N_DUPLICATES): + _alert_id = run.create_metric_range_alert( + name="duplicate_alert_test_metric_range", + range_low=0, + range_high=1, + rule="is inside range", + metric="x" + ) + time.sleep(1) + assert len(RunObject(identifier=run.id).alerts) < 2 + RunObject(identifier=run.id).delete() + + if alert_type == "user" and _alert_id: + UserAlert(identifier=_alert_id).delete() + elif alert_type == "events" and _alert_id: + EventsAlert(identifier=_alert_id).delete() + elif alert_type == "metric_threshold": + MetricsThresholdAlert(identifier=_alert_id).delete() + else: + MetricsRangeAlert(identifier=_alert_id).delete() + From 82df16ca6fd6462fdc2a3e12ce31c81edbfa0524 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristian=20Zar=C4=99bski?= Date: Tue, 31 Mar 2026 11:49:31 +0100 Subject: [PATCH 2/6] Fix offline mode --- simvue/run.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/simvue/run.py b/simvue/run.py index 1b09d417..8fa84a2f 100644 --- a/simvue/run.py +++ b/simvue/run.py @@ -17,6 +17,7 @@ import datetime import os import multiprocessing + import pydantic import re import sys @@ -71,6 +72,7 @@ Grid, ) + try: from typing import Self except ImportError: @@ -2202,7 +2204,9 @@ def add_alerts( def _check_if_alert_exists(self, alert: "AlertBase") -> str | None: """Check if an existing alert matches definition.""" # If the alert already exists just add the existing one - for _id, _existing_alert in Alert.get(): + for _id, _existing_alert in Alert.get( + offline=self._user_config.run.mode == "offline" + ): if _existing_alert == alert: return _id return None From b0bba9c0d864a82ef21c65ea17197fe1c2b8bfdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristian=20Zar=C4=99bski?= Date: Tue, 31 Mar 2026 13:00:47 +0100 Subject: [PATCH 3/6] Use threading.Event to terminate events --- examples/Geant4/geant4_simvue.py | 9 ++------- simvue/executor.py | 9 +++++---- simvue/run.py | 14 +++++++++++--- tests/functional/test_executor.py | 11 +++++------ 4 files changed, 23 insertions(+), 20 deletions(-) diff --git a/examples/Geant4/geant4_simvue.py b/examples/Geant4/geant4_simvue.py index 17ddb0c7..dded4bbf 100644 --- a/examples/Geant4/geant4_simvue.py +++ b/examples/Geant4/geant4_simvue.py @@ -6,16 +6,11 @@ monitoring the yield of key particles of interest """ -import multiparser -import multiparser.parsing.file as mp_file_parse +import threading import simvue -import uproot -import multiprocessing import typing import click import pathlib -import os -import tempfile from particle import Particle @@ -77,7 +72,7 @@ def root_file_parser( for i in range(events): # Create new multiprocessing Trigger which will register when the simulation is complete - _trigger = multiprocessing.Event() + _trigger = threading.Event() if i % 10 == 0: click.secho( diff --git a/simvue/executor.py b/simvue/executor.py index b5e7f0aa..99ac9148 100644 --- a/simvue/executor.py +++ b/simvue/executor.py @@ -13,7 +13,6 @@ import logging import multiprocessing.synchronize import sys -import multiprocessing import threading import os import shutil @@ -49,7 +48,7 @@ def _execute_process( command: list[str], runner_name: str, completion_callback: CompletionCallback | None = None, - completion_trigger: multiprocessing.synchronize.Event | None = None, + completion_trigger: threading.Event | None = None, environment: dict[str, str] | None = None, cwd: pathlib.Path | None = None, ) -> tuple[subprocess.Popen, threading.Thread | None]: @@ -179,7 +178,9 @@ def add_process( env: dict[str, str] | None = None, cwd: pathlib.Path | None = None, completion_callback: CompletionCallback | None = None, - completion_trigger: multiprocessing.synchronize.Event | None = None, + completion_trigger: threading.Event + | multiprocessing.synchronize.Event + | None = None, **kwargs, ) -> None: """Add a process to be executed to the executor. @@ -230,7 +231,7 @@ def callback_function(status_code: int, std_out: str, std_err: str) -> None: working directory to execute the process within completion_callback : typing.Callable | None, optional callback to run when process terminates - completion_trigger : multiprocessing.Event | None, optional + completion_trigger : threading.Event | None, optional this trigger event is set when the processes completes (not supported on Windows) """ pos_args = list(args) diff --git a/simvue/run.py b/simvue/run.py index 8fa84a2f..a160fa75 100644 --- a/simvue/run.py +++ b/simvue/run.py @@ -13,10 +13,10 @@ import multiprocessing.synchronize import shlex import threading +import warnings import humanfriendly import datetime import os -import multiprocessing import pydantic import re @@ -820,7 +820,9 @@ def add_process( completion_callback: typing.Optional[ typing.Callable[[int, str, str], None] ] = None, - completion_trigger: multiprocessing.synchronize.Event | None = None, + completion_trigger: threading.Event + | multiprocessing.synchronize.Event + | None = None, env: dict[str, str] | None = None, cwd: pathlib.Path | None = None, **cmd_kwargs, @@ -883,7 +885,7 @@ def callback_function(status_code: int, std_out: str, std_err: str) -> None: ... you should provide it as such and perform the upload manually, by default None completion_callback : typing.Callable | None, optional callback to run when process terminates (not supported on Windows) - completion_trigger : multiprocessing.Event | None, optional + completion_trigger : threading.Event | None, optional this trigger event is set when the processes completes env : dict[str, str], optional environment variables for process @@ -920,6 +922,12 @@ def callback_function(status_code: int, std_out: str, std_err: str) -> None: ... ) ``` """ + if isinstance(completion_trigger, multiprocessing.synchronize.Event): + warnings.warn( + "Use of a 'multiprocessing.Event' as a termination trigger will be deprecated in v2.5, " + + "use an instance of 'threading.Event' instead." + ) + if platform.system() == "Windows" and completion_trigger: raise RuntimeError( "Use of 'completion_trigger' on Windows based operating systems is unsupported " diff --git a/tests/functional/test_executor.py b/tests/functional/test_executor.py index fcea34c1..375dfe98 100644 --- a/tests/functional/test_executor.py +++ b/tests/functional/test_executor.py @@ -9,8 +9,7 @@ import tempfile import pathlib import os -import multiprocessing -import multiprocessing.synchronize +import threading from simvue.api.objects.folder import Folder from simvue.exception import ObjectNotFoundError @@ -23,7 +22,7 @@ def test_executor_add_process( request: pytest.FixtureRequest ) -> None: import logging - trigger = multiprocessing.Event() + trigger = threading.Event() folder_id = f"{uuid.uuid4()}".split("-")[0] folder = Folder.new(path=f"/simvue_unit_testing/{folder_id}") folder.commit() @@ -79,7 +78,7 @@ def test_executor_multiprocess(request: pytest.FixtureRequest) -> None: def callback(*_, evts=events, ident=i, **__): evts[ident] = True events[i] = False - triggers[i] = multiprocessing.Event() + triggers[i] = threading.Event() callbacks[i] = callback out_file = pathlib.Path(tempd).joinpath(f"out_file_{i}.dat") run.add_process( @@ -183,7 +182,7 @@ def completion_callback(*_, success: dict[str, bool]=success, **__): @pytest.mark.executor @pytest.mark.unix def test_completion_trigger_set(request: pytest.FixtureRequest) -> None: - trigger = multiprocessing.Event() + trigger = threading.Event() folder_id = f"{uuid.uuid4()}".split("-")[0] folder = Folder.new(path=f"/simvue_unit_testing/{folder_id}") folder.commit() @@ -209,7 +208,7 @@ def test_completion_trigger_set(request: pytest.FixtureRequest) -> None: @pytest.mark.executor def test_completion_callbacks_trigger_set(request: pytest.FixtureRequest) -> None: - trigger = multiprocessing.Event() + trigger = threading.Event() def completion_callback(*_, trigger=trigger, **__): trigger.set() From ba9f9288ff1db3a019b6a3f202813787a285cfd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristian=20Zar=C4=99bski?= Date: Tue, 31 Mar 2026 13:27:34 +0100 Subject: [PATCH 4/6] Added duplicate between runs test --- CHANGELOG.md | 1 + tests/functional/test_run_class.py | 100 +++++++++++++++++++++++++---- 2 files changed, 87 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 230c9e58..a52837b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [v2.4.1](https://github.com/simvue-io/python-api/releases/tag/v2.4.1) - 2026-03-31 +- Moved to using `threading.Event` as termination trigger events and added deprecation notice for `multiprocessing.Event`. - Fixed creation of duplicate alerts. - Fixed case where alert specifications being `0` led to incorrect falsification. diff --git a/tests/functional/test_run_class.py b/tests/functional/test_run_class.py index 0b19e687..0057a93b 100644 --- a/tests/functional/test_run_class.py +++ b/tests/functional/test_run_class.py @@ -1598,7 +1598,7 @@ def test_run_environment_metadata(environment: str, mocker: pytest_mock.MockerFi @pytest.mark.parametrize( "alert_type", ("user", "events", "metric_threshold", "metric_range") ) -def test_no_alert_dupes(alert_type: Literal["user", "events", "metric_threshold", "metric_range"]) -> None: +def test_no_alert_dupes_same_run(alert_type: typing.Literal["user", "events", "metric_threshold", "metric_range"]) -> None: """Test there are no duplicate alerts.""" N_DUPLICATES: int = 2 _uuid = f"{uuid.uuid4()}".split("-")[0] @@ -1606,7 +1606,7 @@ def test_no_alert_dupes(alert_type: Literal["user", "events", "metric_threshold" with simvue.Run() as run: run.init( - name="test_no_alert_dupes", + name="test_no_alert_dupes_same_run", folder=f"/simvue_unit_testing/{_uuid}", retention_period=os.environ.get("SIMVUE_TESTING_RETENTION_PERIOD", "2 mins"), running=False, @@ -1618,14 +1618,14 @@ def test_no_alert_dupes(alert_type: Literal["user", "events", "metric_threshold" elif alert_type == "events": for _ in range(N_DUPLICATES): _alert_id = run.create_event_alert( - name="duplicate_alert_test_events", + name="same_run_duplicate_alert_test_events", pattern="no-pattern", ) elif alert_type == "metric_threshold": for _ in range(N_DUPLICATES): _alert_id = run.create_metric_threshold_alert( - name="duplicate_alert_test_metric_threshold", + name="same_run_duplicate_alert_test_metric_threshold", threshold=0, rule="is above", metric="x" @@ -1633,22 +1633,94 @@ def test_no_alert_dupes(alert_type: Literal["user", "events", "metric_threshold" else: for _ in range(N_DUPLICATES): _alert_id = run.create_metric_range_alert( - name="duplicate_alert_test_metric_range", + name="same_run_duplicate_alert_test_metric_range", range_low=0, range_high=1, rule="is inside range", metric="x" ) time.sleep(1) - assert len(RunObject(identifier=run.id).alerts) < 2 + _alerts = RunObject(identifier=run.id).alerts + assert _alerts and len(_alerts) < 2 RunObject(identifier=run.id).delete() - if alert_type == "user" and _alert_id: - UserAlert(identifier=_alert_id).delete() - elif alert_type == "events" and _alert_id: - EventsAlert(identifier=_alert_id).delete() - elif alert_type == "metric_threshold": - MetricsThresholdAlert(identifier=_alert_id).delete() - else: - MetricsRangeAlert(identifier=_alert_id).delete() + with contextlib.suppress(ObjectNotFoundError): + if alert_type == "user" and _alert_id: + UserAlert(identifier=_alert_id).delete() + elif alert_type == "events" and _alert_id: + EventsAlert(identifier=_alert_id).delete() + elif alert_type == "metric_threshold" and _alert_id: + MetricsThresholdAlert(identifier=_alert_id).delete() + elif _alert_id: + MetricsRangeAlert(identifier=_alert_id).delete() + + +@pytest.mark.online +@pytest.mark.parametrize( + "alert_type", ("user", "events", "metric_threshold", "metric_range") +) +def test_no_alert_dupes_different_run(alert_type: typing.Literal["user", "events", "metric_threshold", "metric_range"]) -> None: + """Test there are no duplicate alerts.""" + N_RUNS: int = 2 + _uuid = f"{uuid.uuid4()}".split("-")[0] + _alert_ids: list[str | None] = [] + _run_ids: list[str] = [] + + for run_i in range(N_RUNS): + with simvue.Run() as run: + run.init( + name="test_no_alert_dupes_different_runs", + folder=f"/simvue_unit_testing/{_uuid}_{run_i}", + retention_period=os.environ.get("SIMVUE_TESTING_RETENTION_PERIOD", "2 mins"), + running=False, + visibility="tenant" if os.environ.get("CI") else None, + ) + _run_ids.append(run.id) + if alert_type == "user": + _alert_id = run.create_user_alert(name="diff_run_duplicate_alert_test_user") + _alert_ids.append(_alert_id) + elif alert_type == "events": + _alert_id = run.create_event_alert( + name="diff_run_duplicate_alert_test_events", + pattern="no-pattern", + + ) + _alert_ids.append(_alert_id) + elif alert_type == "metric_threshold": + _alert_id = run.create_metric_threshold_alert( + name="diff_run_duplicate_alert_test_metric_threshold", + threshold=0, + rule="is above", + metric="x" + ) + _alert_ids.append(_alert_id) + else: + _alert_id = run.create_metric_range_alert( + name="diff_run_duplicate_alert_test_metric_range", + range_low=0, + range_high=1, + rule="is inside range", + metric="x" + ) + _alert_ids.append(_alert_id) + time.sleep(1) + + _alerts: list[str] = [] + + for run_id in _run_ids: + _alerts += (RunObject(identifier=run_id).alerts or []) + RunObject(identifier=run_id).delete() + + assert _alerts and len(set(_alerts)) < 2 + + for created_id in _alert_ids: + with contextlib.suppress(ObjectNotFoundError): + if alert_type == "user": + UserAlert(identifier=created_id).delete() + elif alert_type == "events": + EventsAlert(identifier=created_id).delete() + elif alert_type == "metric_threshold": + MetricsThresholdAlert(identifier=created_id).delete() + else: + MetricsRangeAlert(identifier=created_id).delete() From 4a13bca8d36e542c71588bbdb6f23af40775ea86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristian=20Zar=C4=99bski?= Date: Tue, 31 Mar 2026 13:50:29 +0100 Subject: [PATCH 5/6] Fix staging issues when comparing alerts --- simvue/api/objects/alert/base.py | 2 +- simvue/api/objects/base.py | 6 ++++-- simvue/run.py | 7 +++++++ 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/simvue/api/objects/alert/base.py b/simvue/api/objects/alert/base.py index 9dd96730..52450571 100644 --- a/simvue/api/objects/alert/base.py +++ b/simvue/api/objects/alert/base.py @@ -28,7 +28,7 @@ class AlertBase(SimvueObject): """ @classmethod - def new(cls, **kwargs): + def new(cls, read_only: bool = False, **kwargs): """Create a new alert""" pass diff --git a/simvue/api/objects/base.py b/simvue/api/objects/base.py index 0b38df6c..30bf85e8 100644 --- a/simvue/api/objects/base.py +++ b/simvue/api/objects/base.py @@ -513,13 +513,15 @@ def _get_all_objects( else: yield from _generator - def read_only(self, is_read_only: bool) -> None: + def read_only(self, is_read_only: bool, *, clear_staged: bool = True) -> None: """Set whether this object is in read only state. Parameters ---------- is_read_only : bool whether object is read only. + clear_staged : bool, optional + whether to clear staging data, default is True. """ self._read_only = is_read_only @@ -527,7 +529,7 @@ def read_only(self, is_read_only: bool) -> None: # in this context it contains existing data retrieved # from the server/local entry which we dont want to # re-push unnecessarily, then read any locally staged changes - if not self._read_only: + if not self._read_only and clear_staged: self._staging = self._get_local_staged() def commit(self) -> dict | list[dict] | None: diff --git a/simvue/run.py b/simvue/run.py index a160fa75..8c8d81d8 100644 --- a/simvue/run.py +++ b/simvue/run.py @@ -2212,11 +2212,18 @@ def add_alerts( def _check_if_alert_exists(self, alert: "AlertBase") -> str | None: """Check if an existing alert matches definition.""" # If the alert already exists just add the existing one + # set to read only so commit checks are not performed + alert.read_only(True) for _id, _existing_alert in Alert.get( offline=self._user_config.run.mode == "offline" ): if _existing_alert == alert: return _id + + # Return alert to writable so it can be sent to the server + # remembering to not wipe any staged attributes + alert.read_only(False, clear_staged=False) + return None @skip_if_failed("_aborted", "_suppress_errors", None) From bd38b9c7f3e334fd7626372028ba1e968e687e08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristian=20Zar=C4=99bski?= Date: Tue, 31 Mar 2026 14:27:58 +0100 Subject: [PATCH 6/6] Move read-only set in comparison to base object --- simvue/api/objects/alert/base.py | 26 +++++++++++++++++++++++--- simvue/api/objects/alert/events.py | 4 ++-- simvue/api/objects/alert/metrics.py | 8 ++++---- simvue/api/objects/alert/user.py | 4 ++-- simvue/run.py | 7 ------- 5 files changed, 31 insertions(+), 18 deletions(-) diff --git a/simvue/api/objects/alert/base.py b/simvue/api/objects/alert/base.py index 52450571..9aaa2088 100644 --- a/simvue/api/objects/alert/base.py +++ b/simvue/api/objects/alert/base.py @@ -43,9 +43,7 @@ def __init__(self, identifier: str | None = None, **kwargs) -> None: "status", ] - @override - def __eq__(self, other: "AlertBase") -> bool: - """Check if alerts are the same.""" + def _compare_objects(self, other: "AlertBase") -> bool: return all( [ self.name == other.name, @@ -55,6 +53,28 @@ def __eq__(self, other: "AlertBase") -> bool: ] ) + @override + def __eq__(self, other: "AlertBase") -> bool: + """Check if alerts are the same.""" + + # Need to ensure objects are read-only for this + # operation as we do not want staging to alter + _self_is_read_only: bool = self._read_only + _other_is_read_only: bool = other._read_only + self.read_only(True) + other.read_only(True) + + _comparison = self._compare_objects(other) + + # Restore to write allowed unless the input object + # was read-only to begin with + if not _self_is_read_only: + self.read_only(False, clear_staged=False) + if not _other_is_read_only: + other.read_only(False, clear_staged=False) + + return _comparison + def compare(self, other: "AlertBase") -> bool: """Compare this alert to another""" return type(self) is type(other) and self.name == other.name diff --git a/simvue/api/objects/alert/events.py b/simvue/api/objects/alert/events.py index 007a0c53..6963568f 100644 --- a/simvue/api/objects/alert/events.py +++ b/simvue/api/objects/alert/events.py @@ -110,11 +110,11 @@ def new( return _alert @override - def __eq__(self, other: "AlertBase") -> bool: + def _compare_objects(self, other: "AlertBase") -> bool: """Compare Events Alerts.""" if not isinstance(other, EventsAlert): return False - return super().__eq__(other) and self.alert == other.alert + return super()._compare_objects(other) and self.alert == other.alert class EventAlertDefinition: diff --git a/simvue/api/objects/alert/metrics.py b/simvue/api/objects/alert/metrics.py index 5c8c2f92..7eef0e7b 100644 --- a/simvue/api/objects/alert/metrics.py +++ b/simvue/api/objects/alert/metrics.py @@ -140,10 +140,10 @@ def new( return _alert @override - def __eq__(self, other: "AlertBase") -> bool: + def _compare_objects(self, other: "AlertBase") -> bool: if not isinstance(other, MetricsThresholdAlert): return False - return super().__eq__(other) and self.alert == other.alert + return super()._compare_objects(other) and self.alert == other.alert class MetricsRangeAlert(AlertBase): @@ -181,11 +181,11 @@ def __init__(self, identifier: str | None = None, **kwargs) -> None: ] @override - def __eq__(self, other: "AlertBase") -> bool: + def _compare_objects(self, other: "AlertBase") -> bool: """Compare two MetricRangeAlerts""" if not isinstance(other, MetricsRangeAlert): return False - return super().__eq__(other) and self.alert == other.alert + return super()._compare_objects(other) and self.alert == other.alert @classmethod @pydantic.validate_call diff --git a/simvue/api/objects/alert/user.py b/simvue/api/objects/alert/user.py index 44786a03..94e2e003 100644 --- a/simvue/api/objects/alert/user.py +++ b/simvue/api/objects/alert/user.py @@ -90,10 +90,10 @@ def new( return _alert @override - def __eq__(self, other: "AlertBase") -> bool: + def _compare_objects(self, other: "AlertBase") -> bool: if not isinstance(other, UserAlert): return False - return super().__eq__(other) + return super()._compare_objects(other) @classmethod def get( diff --git a/simvue/run.py b/simvue/run.py index 8c8d81d8..a160fa75 100644 --- a/simvue/run.py +++ b/simvue/run.py @@ -2212,18 +2212,11 @@ def add_alerts( def _check_if_alert_exists(self, alert: "AlertBase") -> str | None: """Check if an existing alert matches definition.""" # If the alert already exists just add the existing one - # set to read only so commit checks are not performed - alert.read_only(True) for _id, _existing_alert in Alert.get( offline=self._user_config.run.mode == "offline" ): if _existing_alert == alert: return _id - - # Return alert to writable so it can be sent to the server - # remembering to not wipe any staged attributes - alert.read_only(False, clear_staged=False) - return None @skip_if_failed("_aborted", "_suppress_errors", None)