From 5ea3613b787deaf3d87b6b9d694715ede317f0b5 Mon Sep 17 00:00:00 2001 From: Craig McChesney Date: Sat, 26 Sep 2026 11:23:03 -0600 Subject: [PATCH 1/2] Add helpers for detecting open-ended activations (#26) activation_is_open() and activation_end_time() read an activation saved without end_time back. Reading endTime directly on an open record does not fail but yields a zero Timestamp (1970), and carrying that forward as end_time= in a re-save gets the save rejected. activation_end_time() returns None instead, so it is also the correct carry-forward. Module-level functions rather than a result property, so they cover get, query, iterate, and get_active_configurations() alike. Presence is the only test: an endTime present with value 0 is closed, not open. The cookbook adopts them, fixes the recipe that printed 0 for an open interval, completes the getActiveConfigurations match rule, and adds a recipe for finding the open activation without its id. Closes osprey-dcs/dp-python-lib#26 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PgJ3cRwMrh8UzxzZwraLCT --- .dev/tools/check-cookbook-snippets.py | 2 + CLAUDE.md | 8 +- doc/cookbook/machine-configuration.md | 47 ++++++++-- doc/release-notes/NEXT.md | 25 ++++++ src/dp_python_lib/client/__init__.py | 4 + .../client/machine_config_client.py | 40 +++++++++ .../test_machine_config_client_integration.py | 9 ++ .../test_machine_config_activation_client.py | 85 +++++++++++++++++++ 8 files changed, 213 insertions(+), 7 deletions(-) diff --git a/.dev/tools/check-cookbook-snippets.py b/.dev/tools/check-cookbook-snippets.py index 59924be..ac74d85 100755 --- a/.dev/tools/check-cookbook-snippets.py +++ b/.dev/tools/check-cookbook-snippets.py @@ -87,6 +87,8 @@ ConfigurationActivationQuery, ConfigurationActivationQuery as CA, to_timestamp, + activation_is_open, + activation_end_time, SaveConfigurationRequestParams, SaveConfigurationActivationRequestParams, QueryClient, diff --git a/CLAUDE.md b/CLAUDE.md index cd97d2d..65a3178 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -234,7 +234,7 @@ plan documents one change, `CLAUDE.md` documents the invariant it established. - `src/dp_python_lib/client/ingestion_client.py` - Ingestion service client with methods like `register_provider()` - `src/dp_python_lib/client/annotation_client.py` - Annotation service facade; groups feature-scoped clients sharing the one `DpAnnotationService` channel (`.pv_metadata`, `.machine_config`, `.sample_status`, `.datasets`, `.annotations`, `.export` — every implemented `DpAnnotationService` feature area) - `src/dp_python_lib/client/pv_metadata_client.py` - PV metadata client (`save_pv_metadata()`, `get_pv_metadata()`, `query_pv_metadata()`, `iter_pv_metadata()`, `delete_pv_metadata()`) plus the `PvMetadataQuery` (`Q`) criterion helpers -- `src/dp_python_lib/client/machine_config_client.py` - Machine configuration client covering both configurations (`save_configuration()`, `get_configuration()`, `query_configurations()`, `iter_configurations()`, `delete_configuration()`) and their temporal activations (`save_configuration_activation()`, `get_configuration_activation()`, `query_configuration_activations()`, `iter_configuration_activations()`, `delete_configuration_activation()`, `get_active_configurations()`). Includes the `ConfigurationQuery` (`C`) and `ConfigurationActivationQuery` (`CA`) criterion helpers. The shared time converters it used to own now live in `time_conversions.py`. Get/delete activation take a composite key (`client_activation_id` XOR `configuration_name`+`start_time`). Activation `end_time` is optional — omit it for an open-ended activation ("still in effect"); the field is then genuinely absent on the wire +- `src/dp_python_lib/client/machine_config_client.py` - Machine configuration client covering both configurations (`save_configuration()`, `get_configuration()`, `query_configurations()`, `iter_configurations()`, `delete_configuration()`) and their temporal activations (`save_configuration_activation()`, `get_configuration_activation()`, `query_configuration_activations()`, `iter_configuration_activations()`, `delete_configuration_activation()`, `get_active_configurations()`). Includes the `ConfigurationQuery` (`C`) and `ConfigurationActivationQuery` (`CA`) criterion helpers. The shared time converters it used to own now live in `time_conversions.py`. Get/delete activation take a composite key (`client_activation_id` XOR `configuration_name`+`start_time`). Activation `end_time` is optional — omit it for an open-ended activation ("still in effect"); the field is then genuinely absent on the wire. Read it back with the module-level `activation_is_open()` / `activation_end_time()` (#26), which work on an activation from any read path: reading `.endTime` directly on an open record silently yields a zero `Timestamp` (1970), and passing that back as `end_time=` in a re-save gets the save rejected. `activation_end_time()` returns `None` for an open record, so `end_time=activation_end_time(current)` is the correct carry-forward. Presence is the only test: an `endTime` present with value 0 is closed, not open - `src/dp_python_lib/client/sample_status_client.py` - Sample status client (`save_sample_statuses()`, `query_sample_statuses()`, `iter_sample_statuses()`, `iter_sample_statuses_stream()`, `delete_sample_statuses()`) plus the `SampleStatusColumn` / `SampleStatusFrame` construction classes. The `sampling_clock()` / `timestamp_list()` axis builders now live in `data_frame.py` (issue #6 Phase 2, once calculations frames became a second caller) and are re-exported here, so existing imports are unaffected. A status's identity key is `(pvName, timestamp, domain, layer)`; `delete_sample_statuses()` requires either `pv_names` or an explicit `all_pvs=True` opt-in for the destructive wildcard - `src/dp_python_lib/client/sample_status_conversions.py` - Per-sample expansion of query results (no optional extras required): `expand_data_timestamps()` (SamplingClock positions computed in **integer nanoseconds**, never float seconds — the exact-match contract depends on it), `bucket_to_rows()` / `buckets_to_rows()` / `iter_rows()` yielding `SampleStatusRow` objects with absent confidence/reason surfaced as `None` rather than fabricated `0.0`/`""` - `src/dp_python_lib/client/dataset_client.py` - DataSet client (`save_dataset()`, `get_dataset()`, `query_datasets()`, `iter_datasets()`, `delete_dataset()`, plus the `get_datasets(ids)` batch fetch that avoids the annotation-listing N+1) with the `DataSetQuery` (`DS`) criterion helpers and the `data_block()` builder. `data_block()` is the only place `begin < end` is checked — the server does not @@ -495,6 +495,8 @@ from dp_python_lib.client import ( SaveConfigurationActivationRequestParams, ConfigurationQuery as C, ConfigurationActivationQuery as CA, + activation_is_open, + activation_end_time, ) client = MldpClient() @@ -527,7 +529,7 @@ mc.get_configuration_activation(configuration_name="beamline-optics", start_time # query/iterate activations (raises RuntimeError on a page error) for a in mc.iter_configuration_activations([CA.configuration_name(["beamline-optics"])]): - print(a.clientActivationId) + print(a.clientActivationId, "open" if activation_is_open(a) else activation_end_time(a).epochSeconds) # what is active right now? (pass a timestamp for a historical instant) active = mc.get_active_configurations().configuration_activations @@ -539,6 +541,8 @@ mc.delete_configuration("beamline-optics") Notes: - `to_timestamp()` (also exported) is the shared time converter; naive datetimes raise `ValueError`. - Composite-key get/delete require exactly one key form (id XOR name+start_time); violations raise `ValueError`. +- Never read `activation.endTime` without a presence check: on an open-ended activation it is a zero `Timestamp` + (1970-01-01), not an error or `None`. Use `activation_is_open()` / `activation_end_time()`. - `ConfigurationQuery` (`C`) criteria: `name`/`category`/`tags`/`attributes`/`parent`. `ConfigurationActivationQuery` (`CA`) criteria: `timestamp`/`time_range`/`configuration_name`/`client_activation_id`/`category`/`tags`/`attributes`. Each helper raises `ValueError` on empty inputs. diff --git a/doc/cookbook/machine-configuration.md b/doc/cookbook/machine-configuration.md index 23071fd..37e8d6e 100644 --- a/doc/cookbook/machine-configuration.md +++ b/doc/cookbook/machine-configuration.md @@ -21,6 +21,8 @@ from dp_python_lib.client import ( ConfigurationQuery as C, ConfigurationActivationQuery as CA, to_timestamp, + activation_is_open, + activation_end_time, ) ``` @@ -180,7 +182,8 @@ machine_config.save_configuration_activation(SaveConfigurationActivationRequestP after its `start_time`. To close it, re-save the record with the same `client_activation_id` and a real `end_time` — see the next section. -To test whether a record you have read back is still open, check the field directly: +To test whether a record you have read back is still open, use `activation_is_open()`, and read +its end time with `activation_end_time()`, which returns `None` for an open record: ```python # cookbook:partial @@ -190,9 +193,13 @@ read = machine_config.get_configuration_activation(client_activation_id="act-ope activation = read.configuration_activation assert activation is not None -still_open = not activation.HasField("endTime") +still_open = activation_is_open(activation) # True +ends_at = activation_end_time(activation) # None while still open ``` +Do not read `activation.endTime` directly. On an open record it does not fail: it returns a zero +`Timestamp`, which is 1970-01-01, is truthy, and is not `None`, so no ordinary check catches it. + ## Closing one activation and opening the next When the machine changes configuration at time `t`, close the current interval at `t` and open @@ -242,7 +249,36 @@ Two things to get right: than a second, overlapping activation record. - **Copy every field forward.** `save_*` is full-replace, so omitting `description`, `tags`, or `attributes` erases them. Note that `start_time` accepts the `common.Timestamp` you read back - directly — no conversion needed. + directly — no conversion needed. For a re-save that is *not* closing the activation (a retag, + say), carry the end time forward with `end_time=activation_end_time(current)`, never + `end_time=current.endTime`. The helper passes `None` through, so an open activation stays open; + the raw field would send the 1970 value, and the server rejects the save because the end + precedes the start. + +### When you do not have the activation's id + +A live bridge often knows only which configuration is in effect, not the id of its open +activation. There is no server-side criterion for "open", so query by configuration name and +filter: + +```python +# cookbook:partial +machine_config = client.annotation.machine_config + +open_activations = [ + a for a in machine_config.iter_configuration_activations([CA.configuration_name(["cxi-production"])]) + if activation_is_open(a) +] +if len(open_activations) > 1: + raise RuntimeError(f"{len(open_activations)} open activations for cxi-production; expected at most one") +current = open_activations[0] if open_activations else None # None: nothing to close, go to step 3 +``` + +Zero open records is normal: nothing is in effect, so skip the close and go straight to step 3. +More than one should not happen, since the server rejects an activation overlapping another with +the same configuration name or category. It can arise only if two saves race past that check, +which is not atomic. Raise rather than pick one: closing either leaves the other open and still +overlapping. ### Late reports @@ -272,7 +308,7 @@ for activation in result.configuration_activations: ``` `get_active_configurations()` returns every activation whose interval covers that instant — -`startTime <= t` and `endTime > t`. Several may be active at once when they belong to different +`startTime <= t`, and either `endTime > t` or no `endTime` at all (an open-ended activation). Several may be active at once when they belong to different categories. Called with no argument, it answers **"what is active right now"**: @@ -328,7 +364,8 @@ for activation in client.annotation.machine_config.iter_configuration_activation for activation in client.annotation.machine_config.iter_configuration_activations([ CA.configuration_name(["cxi-production"]), ]): - print(activation.startTime.epochSeconds, activation.endTime.epochSeconds) + ends_at = activation_end_time(activation) + print(activation.startTime.epochSeconds, ends_at.epochSeconds if ends_at is not None else "still in effect") ``` ### All the beam time an experiment received diff --git a/doc/release-notes/NEXT.md b/doc/release-notes/NEXT.md index dfaf6df..3b654d0 100644 --- a/doc/release-notes/NEXT.md +++ b/doc/release-notes/NEXT.md @@ -29,6 +29,7 @@ person cutting the release has any reason to re-read. - [Type checking in CI (#30)](#type-checking-in-ci-issue-30) - [Ready for typed gRPC stubs (#61)](#ready-for-typed-grpc-stubs-issue-61) - [Environment variables override the config file (#19)](#environment-variables-override-the-config-file-issue-19) +- [Detecting open-ended activations (#26)](#detecting-open-ended-activations-issue-26) - [Cutting the release](#cutting-the-release) --- @@ -114,6 +115,30 @@ Two smaller consequences of the same fix: See [#19](https://github.com/osprey-dcs/dp-python-lib/issues/19) and the configuration priority section of [`doc/cookbook/connecting.md`](https://github.com/osprey-dcs/dp-python-lib/blob/main/doc/cookbook/connecting.md#configuration-priority). +## Detecting open-ended activations (Issue #26) + +Two new helpers, exported from `dp_python_lib.client`, read an open-ended configuration activation +back (one saved without `end_time`, meaning "still in effect"): + +- **`activation_is_open(activation)`** reports whether an activation has no end time. +- **`activation_end_time(activation)`** returns the end time as a `common.Timestamp`, or `None` + for an open activation. + +Both work on an activation from any read path: get, query, iterate, or +`get_active_configurations()`. They exist because reading `activation.endTime` directly on an open +record does not fail. It returns a zero `Timestamp`, 1970-01-01, which is truthy and not `None`, +so no ordinary check notices. Passing that value back as `end_time=` in a re-save turns the open +activation into one the server rejects for ending before it starts; `end_time=activation_end_time(current)` +carries it forward correctly. + +This is additive; nothing needs to change on upgrade. **If you copied the "Every interval a +configuration was in effect" recipe** from +[`doc/cookbook/machine-configuration.md`](https://github.com/osprey-dcs/dp-python-lib/blob/main/doc/cookbook/machine-configuration.md), +it printed `0` as the end of an open interval; the recipe now uses `activation_end_time()`. The +cookbook also gains a recipe for finding the open activation when you do not have its id. + +See [#26](https://github.com/osprey-dcs/dp-python-lib/issues/26). + ## Installing ```bash diff --git a/src/dp_python_lib/client/__init__.py b/src/dp_python_lib/client/__init__.py index 1d20093..5f0b8b9 100644 --- a/src/dp_python_lib/client/__init__.py +++ b/src/dp_python_lib/client/__init__.py @@ -67,6 +67,8 @@ SaveConfigurationActivationRequestParams, SaveConfigurationApiResult, SaveConfigurationRequestParams, + activation_end_time, + activation_is_open, ) from dp_python_lib.client.mldp_client import MldpClient from dp_python_lib.client.pv_metadata_client import ( @@ -163,6 +165,8 @@ "SaveSampleStatusesApiResult", "SaveSampleStatusesRequestParams", "TimestampInput", + "activation_end_time", + "activation_is_open", "bool_column", "calculations", "calculations_source", diff --git a/src/dp_python_lib/client/machine_config_client.py b/src/dp_python_lib/client/machine_config_client.py index fdb8856..c59e83c 100644 --- a/src/dp_python_lib/client/machine_config_client.py +++ b/src/dp_python_lib/client/machine_config_client.py @@ -268,6 +268,45 @@ def attributes( return criterion +# Reading an open-ended activation back (issue #26). The server leaves endTime genuinely absent on every read path +# (get, query, and getActiveConfigurations share one converter), so presence is the whole test. These take a +# ConfigurationActivation rather than a result object so they cover all four read paths with one definition. + + +def activation_is_open(activation: common_pb2.ConfigurationActivation) -> bool: + """ + Reports whether an activation is open-ended, meaning its configuration is still in effect. + + An open-ended activation has no endTime at all. Test it with this (or activation_end_time()) rather than by + reading activation.endTime: on an open record that read does not fail, but returns a zero Timestamp + (1970-01-01), which is truthy, is not None, and converts to 0 epoch nanoseconds. + + An endTime that is present with value zero is a real end time, not an open one; only absence means open. + + :param activation: An activation from any read path: get, query, iterate, or get_active_configurations(). + :return: True if the activation has no endTime. + """ + return not activation.HasField("endTime") + + +def activation_end_time(activation: common_pb2.ConfigurationActivation) -> common_pb2.Timestamp | None: + """ + Returns an activation's endTime, or None if the activation is open-ended. + + Use this instead of reading activation.endTime directly, which on an open record silently yields a zero + Timestamp (1970-01-01) rather than signalling absence. + + The result is the field's own Timestamp, which end_time= accepts as-is, so this is also the correct way to + carry the end time forward in a full-replace re-save: end_time=activation_end_time(current) keeps an open + activation open, whereas end_time=current.endTime would send the 1970 value and be rejected by the server as + ending before it starts. + + :param activation: An activation from any read path: get, query, iterate, or get_active_configurations(). + :return: The activation's endTime (the message's own sub-message, not a copy), or None if it is open-ended. + """ + return None if activation_is_open(activation) else activation.endTime + + class SaveConfigurationRequestParams: """ Encapsulates client parameters for a call to the saveConfiguration() API method. @@ -438,6 +477,7 @@ def __init__( :param start_time: Start of the activation interval (tz-aware datetime, epoch seconds, or common.Timestamp). :param end_time: End of the activation interval (tz-aware datetime, epoch seconds, or common.Timestamp). Omit or pass None for an open-ended activation, meaning the configuration is still in effect. + Read it back with activation_is_open() / activation_end_time(), not by reading endTime directly. :param client_activation_id: Optional client-supplied identifier for the activation. :param description: Human-readable description of the activation. :param tags: List of tags (keywords) describing the activation. diff --git a/tests/integration/test_machine_config_client_integration.py b/tests/integration/test_machine_config_client_integration.py index 588adb8..1d9019c 100644 --- a/tests/integration/test_machine_config_client_integration.py +++ b/tests/integration/test_machine_config_client_integration.py @@ -15,6 +15,8 @@ ConfigurationQuery, SaveConfigurationActivationRequestParams, SaveConfigurationRequestParams, + activation_end_time, + activation_is_open, ) from dp_python_lib.client.mldp_client import MldpClient @@ -359,6 +361,9 @@ def test_open_ended_activation_round_trip(self): activation.HasField("endTime"), "an open-ended activation must round-trip with endTime absent, not defaulted to zero", ) + # The helpers must agree with the raw check above, which is the one that verifies the wire. + self.assertTrue(activation_is_open(activation)) + self.assertIsNone(activation_end_time(activation)) self.logger.info("Open-ended activation round-tripped with endTime absent") # --- an open-ended activation is active at any instant at or after start_time --- @@ -401,6 +406,10 @@ def test_open_ended_activation_round_trip(self): self.assertIsNotNone(closed, "getConfigurationActivation should return the closed record") self.assertTrue(closed.HasField("endTime"), "closing the activation should set endTime") self.assertEqual(closed.endTime.epochSeconds, end_time) + self.assertFalse(activation_is_open(closed)) + closed_end = activation_end_time(closed) + self.assertIsNotNone(closed_end) + self.assertEqual(closed_end.epochSeconds, end_time) self.logger.info("Closed the open-ended activation at %d", end_time) # --- once closed, the far-future probe no longer sees it --- diff --git a/tests/unit/test_machine_config_activation_client.py b/tests/unit/test_machine_config_activation_client.py index 32fb111..2431246 100644 --- a/tests/unit/test_machine_config_activation_client.py +++ b/tests/unit/test_machine_config_activation_client.py @@ -16,6 +16,8 @@ MachineConfigClient, QueryConfigurationActivationsApiResult, SaveConfigurationActivationRequestParams, + activation_end_time, + activation_is_open, ) from dp_python_lib.grpc import annotation_pb2, common_pb2 @@ -194,6 +196,89 @@ def test_end_time_zero_is_not_treated_as_absent(self): self.assertEqual(request.endTime.epochSeconds, 0) +class TestActivationOpenEndedHelpers(unittest.TestCase): + """activation_is_open() / activation_end_time(): reading an open-ended activation back (#26).""" + + @staticmethod + def _activation(activation_id="act-1", end_seconds=None): + activation = common_pb2.ConfigurationActivation( + configurationName="cfg-1", + clientActivationId=activation_id, + startTime=common_pb2.Timestamp(epochSeconds=100), + ) + if end_seconds is not None: + activation.endTime.CopyFrom(common_pb2.Timestamp(epochSeconds=end_seconds)) + return activation + + def test_open_ended(self): + activation = self._activation() + self.assertTrue(activation_is_open(activation)) + self.assertIsNone(activation_end_time(activation)) + + def test_bounded(self): + activation = self._activation(end_seconds=200) + self.assertFalse(activation_is_open(activation)) + self.assertEqual(activation_end_time(activation), common_pb2.Timestamp(epochSeconds=200)) + + def test_epoch_zero_end_time_is_not_open(self): + """Presence is the only test: an endTime present with value 0 is a real end time, not an open one.""" + activation = self._activation(end_seconds=0) + self.assertFalse(activation_is_open(activation)) + end_time = activation_end_time(activation) + self.assertIsNotNone(end_time) + self.assertEqual(end_time.epochSeconds, 0) + + def test_reading_end_time_does_not_close_an_open_record(self): + """Reading .endTime on an open record yields a zero Timestamp but must not set presence.""" + activation = self._activation() + self.assertEqual(activation.endTime.epochSeconds, 0) # the silent 1970 value the helpers exist to avoid + self.assertTrue(activation_is_open(activation)) + self.assertIsNone(activation_end_time(activation)) + + def test_end_time_round_trips_into_a_resave(self): + client = MachineConfigClient(Mock()) + activation = self._activation(end_seconds=200) + params = SaveConfigurationActivationRequestParams( + configuration_name=activation.configurationName, + start_time=activation.startTime, + end_time=activation_end_time(activation), + ) + request = client._build_save_configuration_activation_request(params) + self.assertTrue(request.HasField("endTime")) + self.assertEqual(request.endTime.epochSeconds, 200) + + def test_carrying_an_open_record_forward_keeps_it_open(self): + """end_time=activation_end_time(open) re-saves as open; end_time=open.endTime would send 1970.""" + client = MachineConfigClient(Mock()) + activation = self._activation() + params = SaveConfigurationActivationRequestParams( + configuration_name=activation.configurationName, + start_time=activation.startTime, + end_time=activation_end_time(activation), + ) + request = client._build_save_configuration_activation_request(params) + self.assertFalse(request.HasField("endTime")) + + def test_filters_a_query_page_to_the_open_records(self): + """The live-bridge case: find the open activation(s) among a query page's mixed results.""" + response = annotation_pb2.QueryConfigurationActivationsResponse() + response.queryConfigurationActivationsResult.configurationActivations.extend( + [ + self._activation("closed-1", end_seconds=200), + self._activation("open-1"), + self._activation("closed-2", end_seconds=0), + ] + ) + client = MachineConfigClient(Mock()) + client._stub = Mock() + client._stub.queryConfigurationActivations.return_value = response + + result = client._send_query_configuration_activations(annotation_pb2.QueryConfigurationActivationsRequest()) + self.assertFalse(result.result_status.is_error) + open_ids = [a.clientActivationId for a in result.configuration_activations if activation_is_open(a)] + self.assertEqual(open_ids, ["open-1"]) + + class TestActivationKeyValidation(unittest.TestCase): def setUp(self): self.client = MachineConfigClient(Mock()) From 0128fc97a0d51d48f6caf88777143e05bca19ec9 Mon Sep 17 00:00:00 2001 From: Craig McChesney Date: Sat, 26 Sep 2026 12:45:07 -0600 Subject: [PATCH 2/2] Address review of PR #65 - Cookbook: the "no activation id" recipe now queries by the incoming configuration's category plus CA.timestamp(changeover), instead of paging a configuration's whole history by name and filtering to open records. That matches the server's overlap check, which is by category as well as name, and also catches a bounded activation whose scheduled end is after the changeover. - CLAUDE.md: carry-forward snippet narrows activation_end_time() with `is not None`, so it stays clean once typed stubs land. - NEXT.md: correct the description of the rejected re-save. - Rewrap the get_active_configurations() paragraph; note the recipe revision in plan/tickets/26/plan.md; fix a test docstring. Refs #26 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01PgJ3cRwMrh8UzxzZwraLCT --- CLAUDE.md | 4 +- doc/cookbook/machine-configuration.md | 49 +++++++++++++------ doc/release-notes/NEXT.md | 9 ++-- plan/tickets/26/plan.md | 11 +++++ .../test_machine_config_activation_client.py | 2 +- 5 files changed, 53 insertions(+), 22 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 65a3178..7f3292d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -529,10 +529,12 @@ mc.get_configuration_activation(configuration_name="beamline-optics", start_time # query/iterate activations (raises RuntimeError on a page error) for a in mc.iter_configuration_activations([CA.configuration_name(["beamline-optics"])]): - print(a.clientActivationId, "open" if activation_is_open(a) else activation_end_time(a).epochSeconds) + ends_at = activation_end_time(a) # None while open; never read a.endTime directly + print(a.clientActivationId, ends_at.epochSeconds if ends_at is not None else "open") # what is active right now? (pass a timestamp for a historical instant) active = mc.get_active_configurations().configuration_activations +open_ended = [a for a in active if activation_is_open(a)] mc.delete_configuration_activation(client_activation_id="act-001") mc.delete_configuration("beamline-optics") diff --git a/doc/cookbook/machine-configuration.md b/doc/cookbook/machine-configuration.md index 37e8d6e..29d375f 100644 --- a/doc/cookbook/machine-configuration.md +++ b/doc/cookbook/machine-configuration.md @@ -257,29 +257,46 @@ Two things to get right: ### When you do not have the activation's id -A live bridge often knows only which configuration is in effect, not the id of its open -activation. There is no server-side criterion for "open", so query by configuration name and -filter: +A live bridge often knows only the configuration it is switching *to*, not the id of the +activation it must close, or even which configuration that activation belongs to. Look it up +by what the server checks. Step 3 is rejected if the new interval overlaps any activation in +the same category, whatever that activation's configuration name. So ask for the activation in +that category that is in effect at the changeover: ```python # cookbook:partial machine_config = client.annotation.machine_config +changeover = datetime(2026, 2, 2, 23, 0, tzinfo=timezone.utc) -open_activations = [ - a for a in machine_config.iter_configuration_activations([CA.configuration_name(["cxi-production"])]) - if activation_is_open(a) -] -if len(open_activations) > 1: - raise RuntimeError(f"{len(open_activations)} open activations for cxi-production; expected at most one") -current = open_activations[0] if open_activations else None # None: nothing to close, go to step 3 +incoming = machine_config.get_configuration("mfx-production") +if incoming.result_status.is_error: + raise RuntimeError(incoming.result_status.message) +assert incoming.configuration is not None +category = incoming.configuration.category + +in_effect = list(machine_config.iter_configuration_activations([ + CA.category([category]), + CA.timestamp(changeover), # in effect at the changeover, open-ended ones included +])) +if len(in_effect) > 1: + raise RuntimeError(f"{len(in_effect)} activations in effect for category {category!r}; expected at most one") +current = in_effect[0] if in_effect else None # None: nothing to close, go to step 3 ``` -Zero open records is normal: nothing is in effect, so skip the close and go straight to step 3. -More than one should not happen, since the server rejects an activation overlapping another with -the same configuration name or category. It can arise only if two saves race past that check, -which is not atomic. Raise rather than pick one: closing either leaves the other open and still +Do not narrow this to open activations with `activation_is_open()`. An activation with a +scheduled end after the changeover blocks step 3 just as an open one does, and step 2 closes +either kind the same way, by setting `end_time` to the changeover. + +Zero results is normal: nothing in that category is in effect, so skip the close and go straight +to step 3. More than one should not happen, since the server rejects an activation overlapping +another in the same category. It can arise only if two saves race past that check, which is not +atomic. Raise rather than pick one: closing either leaves the other in effect and still overlapping. +An activation in the category that *starts* after the changeover is not returned, and step 3 +is still rejected if it overlaps one. That is a scheduled activation, and a real conflict for +a person to resolve, not something for the bridge to close. + ### Late reports If a configuration change is reported after the fact, use the **actual** change time, not the @@ -308,8 +325,8 @@ for activation in result.configuration_activations: ``` `get_active_configurations()` returns every activation whose interval covers that instant — -`startTime <= t`, and either `endTime > t` or no `endTime` at all (an open-ended activation). Several may be active at once when they belong to different -categories. +`startTime <= t`, and either `endTime > t` or no `endTime` at all (an open-ended activation). +Several may be active at once when they belong to different categories. Called with no argument, it answers **"what is active right now"**: diff --git a/doc/release-notes/NEXT.md b/doc/release-notes/NEXT.md index 3b654d0..b0ab122 100644 --- a/doc/release-notes/NEXT.md +++ b/doc/release-notes/NEXT.md @@ -127,15 +127,16 @@ back (one saved without `end_time`, meaning "still in effect"): Both work on an activation from any read path: get, query, iterate, or `get_active_configurations()`. They exist because reading `activation.endTime` directly on an open record does not fail. It returns a zero `Timestamp`, 1970-01-01, which is truthy and not `None`, -so no ordinary check notices. Passing that value back as `end_time=` in a re-save turns the open -activation into one the server rejects for ending before it starts; `end_time=activation_end_time(current)` -carries it forward correctly. +so no ordinary check notices. Passing that value back as `end_time=` in a re-save gets the save +rejected for an end time before the start; `end_time=activation_end_time(current)` carries it +forward correctly. This is additive; nothing needs to change on upgrade. **If you copied the "Every interval a configuration was in effect" recipe** from [`doc/cookbook/machine-configuration.md`](https://github.com/osprey-dcs/dp-python-lib/blob/main/doc/cookbook/machine-configuration.md), it printed `0` as the end of an open interval; the recipe now uses `activation_end_time()`. The -cookbook also gains a recipe for finding the open activation when you do not have its id. +cookbook also gains a recipe for finding the activation to close at a changeover when you do +not have its id. See [#26](https://github.com/osprey-dcs/dp-python-lib/issues/26). diff --git a/plan/tickets/26/plan.md b/plan/tickets/26/plan.md index 9bb57ef..73b7a31 100644 --- a/plan/tickets/26/plan.md +++ b/plan/tickets/26/plan.md @@ -194,6 +194,17 @@ format. two open records for one name can exist only through the race that check's own comment accepts as a v1 limitation: the check and the write are not atomic. The recipe raises rather than choosing one. Closing either would leave the other open and still overlapping. + + *Revised in review of the implementation PR (#65), 2026-09-26.* The shipped recipe does not filter by + configuration name and `activation_is_open()`. A name query pages through the configuration's whole + activation history on every changeover. It also misses the case that actually gets step 3 rejected: + `overlapExists()` checks by category as well as by name, and a bridge switching to a new configuration may + not know the name of the one in effect. The recipe instead reads the incoming configuration's category and + queries `CA.category([category])` with `CA.timestamp(changeover)`. The server's `activationContainsInstantFilter` + counts an absent `endTime` as in effect. It deliberately does not narrow to open records: an activation with + a scheduled end after the changeover blocks step 3 just as an open one does, and step 2 closes either kind. + The zero / one / more-than-one handling is unchanged. An activation starting after the changeover is not + returned, and the recipe leaves that conflict to the server's rejection. - "Copy every field forward" (the list after the closing recipe): add `end_time` to it. A re-save that is not closing the activation carries the end time forward with `end_time=activation_end_time(current)`, never `current.endTime`. Give the one-sentence reason (T4, last bullet). diff --git a/tests/unit/test_machine_config_activation_client.py b/tests/unit/test_machine_config_activation_client.py index 2431246..33fa0ee 100644 --- a/tests/unit/test_machine_config_activation_client.py +++ b/tests/unit/test_machine_config_activation_client.py @@ -260,7 +260,7 @@ def test_carrying_an_open_record_forward_keeps_it_open(self): self.assertFalse(request.HasField("endTime")) def test_filters_a_query_page_to_the_open_records(self): - """The live-bridge case: find the open activation(s) among a query page's mixed results.""" + """The query-path case: find the open activation(s) among a query page's mixed results.""" response = annotation_pb2.QueryConfigurationActivationsResponse() response.queryConfigurationActivationsResult.configurationActivations.extend( [