From 9c33ad767c4370adcc788c117c2935d0ca072500 Mon Sep 17 00:00:00 2001 From: David Leong <116610336+leongdl@users.noreply.github.com> Date: Wed, 22 Jul 2026 14:12:09 -0700 Subject: [PATCH] =?UTF-8?q?feat:=20Add=20WrappedAction.Cancelation.*=20wra?= =?UTF-8?q?p-hook=20template=20variables=20RFC=200008=20follow-up=20(openj?= =?UTF-8?q?d-specifications=20#148):=20expose=20the=20wrapped=20action's?= =?UTF-8?q?=20cancelation=20description=20to=20wrap=20hooks=20so=20a=20wra?= =?UTF-8?q?pper=20(e.g.=20a=20container=20runtime)=20can=20honor=20the=20s?= =?UTF-8?q?ame=20cancelation=20contract=20the=20runtime=20would=20have=20e?= =?UTF-8?q?nforced=20in=20the=20unwrapped=20case:=20-=20WrappedAction.Canc?= =?UTF-8?q?elation.Mode=20(string=3F):=20TERMINATE,=20NOTIFY=5FTHEN=5FTERM?= =?UTF-8?q?INATE,=20or=20null=20when=20the=20wrapped=20action=20declares?= =?UTF-8?q?=20no=20=20=E2=80=94=20deliberately=20distinct=20f?= =?UTF-8?q?rom=20an=20explicit=20TERMINATE.=20-=20WrappedAction.Cancelatio?= =?UTF-8?q?n.NotifyPeriodInSeconds=20(int=3F):=20the=20effective=20grace?= =?UTF-8?q?=20period,=20applying=20the=20Template=20Schemas=205.3.2=20defa?= =?UTF-8?q?ults=20when=20the=20field=20is=20omitted;=20null=20when=20not?= =?UTF-8?q?=20applicable.=20-=20Format-string=20cancelation=20modes=20(FEA?= =?UTF-8?q?TURE=5FBUNDLE=5F1)=20follow=20normal=20format-string=20semantic?= =?UTF-8?q?s:=20any=20format=20string=20is=20statically=20valid,=20the=20r?= =?UTF-8?q?esolved=20value=20is=20checked=20against=20the=20two=20mode=20n?= =?UTF-8?q?ames=20at=20run=20time,=20and=20a=20whole-field=20expression=20?= =?UTF-8?q?resolving=20to=20null=20drops=20the=20cancelation=20object.=20R?= =?UTF-8?q?esolution=20is=20deferred=20to=20run=20time,=20mirroring=20open?= =?UTF-8?q?jd-rs.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com> --- src/openjd/model/_format_strings/_nodes.py | 11 +- src/openjd/model/v2023_09/__init__.py | 2 + src/openjd/model/v2023_09/_model.py | 194 +++++++++++++++--- .../format_strings/test_format_string.py | 21 ++ .../model_v0/format_strings/test_node.py | 30 +++ .../model_v0/v2023_09/test_wrap_actions.py | 105 ++++++++++ 6 files changed, 336 insertions(+), 27 deletions(-) diff --git a/src/openjd/model/_format_strings/_nodes.py b/src/openjd/model/_format_strings/_nodes.py index 0bcb5f3a..7a3308bb 100644 --- a/src/openjd/model/_format_strings/_nodes.py +++ b/src/openjd/model/_format_strings/_nodes.py @@ -66,8 +66,17 @@ def evaluate_to_str(self, *, symtab: SymbolTable, path_format: Any = None) -> st EXPR-backed nodes override this to use the engine's own spec-defined coercion (RFC 0005), so e.g. ``true``/``false``/``null`` and lists render per the specification rather than as Python reprs. + + A ``None`` value interpolates as the empty string, matching the EXPR + engine's null rendering (RFC 0005) — relevant for nullable injected + symbols such as ``WrappedAction.Cancelation.NotifyPeriodInSeconds`` + (RFC 0008 follow-up), which is ``None`` when no notify period + applies. """ - return str(self.evaluate(symtab=symtab, path_format=path_format)) + value = self.evaluate(symtab=symtab, path_format=path_format) + if value is None: + return "" + return str(value) @abstractmethod def __repr__(self) -> str: # pragma: no cover diff --git a/src/openjd/model/v2023_09/__init__.py b/src/openjd/model/v2023_09/__init__.py index 13f81813..68073d87 100644 --- a/src/openjd/model/v2023_09/__init__.py +++ b/src/openjd/model/v2023_09/__init__.py @@ -18,6 +18,7 @@ AttributeCapabilityValue, AttributeRequirement, AttributeRequirementTemplate, + CancelationMethodDeferred, CancelationMethodNotifyThenTerminate, CancelationMethodTerminate, CancelationMode, @@ -113,6 +114,7 @@ "AttributeCapabilityValue", "AttributeRequirement", "AttributeRequirementTemplate", + "CancelationMethodDeferred", "CancelationMethodNotifyThenTerminate", "CancelationMethodTerminate", "CancelationMode", diff --git a/src/openjd/model/v2023_09/_model.py b/src/openjd/model/v2023_09/_model.py index f7d21254..7b1733bd 100644 --- a/src/openjd/model/v2023_09/_model.py +++ b/src/openjd/model/v2023_09/_model.py @@ -15,12 +15,14 @@ field_validator, model_validator, ConfigDict, + Discriminator, StringConstraints, Field, PositiveInt, PositiveFloat, StrictBool, StrictInt, + Tag, ValidationError, ValidationInfo, ) @@ -283,6 +285,31 @@ class CancelationMode(str, Enum): NotifyPeriodType = Annotated[int, Field(ge=1, le=600)] +def _validate_notify_period_value( + v: Any, info: ValidationInfo +) -> Optional[Union[int, FormatString]]: + """Shared notifyPeriodInSeconds validation for + CancelationMethodNotifyThenTerminate and CancelationMethodDeferred.""" + if v is None: + return v + context = cast(Optional[ModelParsingContext], info.context) + if isinstance(v, str): + if context and "FEATURE_BUNDLE_1" not in context.extensions: + # Try to parse as int, fail if not + try: + return int(v) + except ValueError: + raise ValueError( + "notifyPeriodInSeconds as a format string requires the FEATURE_BUNDLE_1 extension." + ) + return validate_int_fmtstring_field(v, ge=1, context=context) + if isinstance(v, int): + if v < 1 or v > 600: + raise ValueError("notifyPeriodInSeconds must be between 1 and 600") + return v + return v + + class CancelationMethodNotifyThenTerminate(OpenJDModel_v2023_09): """Notify-then-terminate cancelation mode for an Action. @@ -323,24 +350,7 @@ class CancelationMethodNotifyThenTerminate(OpenJDModel_v2023_09): def _validate_notify_period( cls, v: Any, info: ValidationInfo ) -> Optional[Union[int, FormatString]]: - if v is None: - return v - context = cast(Optional[ModelParsingContext], info.context) - if isinstance(v, str): - if context and "FEATURE_BUNDLE_1" not in context.extensions: - # Try to parse as int, fail if not - try: - return int(v) - except ValueError: - raise ValueError( - "notifyPeriodInSeconds as a format string requires the FEATURE_BUNDLE_1 extension." - ) - return validate_int_fmtstring_field(v, ge=1, context=context) - if isinstance(v, int): - if v < 1 or v > 600: - raise ValueError("notifyPeriodInSeconds must be between 1 and 600") - return v - return v + return _validate_notify_period_value(v, info) class CancelationMethodTerminate(OpenJDModel_v2023_09): @@ -357,6 +367,130 @@ class CancelationMethodTerminate(OpenJDModel_v2023_09): mode: Literal[CancelationMode.TERMINATE] +class CancelationMethodDeferred(OpenJDModel_v2023_09): + """A cancelation whose ``mode`` is a format string, resolved at run + time (Template Schemas 5.3, FEATURE_BUNDLE_1 extension). + + What is the problem this solves? + + Format strings in general are *already* delay-processed: when a template + says ``args: ["{{WrappedAction.Command}}"]``, the parser just stores + "this is a format string" and the value gets resolved much later, inside + a running session, right before the action launches — that's when the + runtime seeds the ``WrappedAction.*`` variables from the action being + wrapped. "Resolve later" is the normal pipeline for every other field. + + ``mode`` is different because it isn't a normal value field — it's the + *schema selector*. The parser needs to know TERMINATE vs + NOTIFY_THEN_TERMINATE at parse time to decide what shape of object it's + even reading (only one of them allows ``notifyPeriodInSeconds``). So the + "which shape?" decision happens at parse time, but a forwarded value + like ``mode: "{{WrappedAction.Cancelation.Mode}}"`` only exists at run + time — that mismatch made round-trip cancelation forwarding in RFC 0008 + wrap hooks impossible (pydantic's discriminated union rejected the + template with "does not match any of the expected tags"). + + The fix is this class: the parser accepts a format string in ``mode`` + as a third, "decided later" state (gated on the FEATURE_BUNDLE_1 + extension), and the shape decision moves to resolution time, right + before the action runs: + + 1. The runtime seeds ``WrappedAction.Cancelation.Mode`` from the + wrapped action (``"TERMINATE"``, ``"NOTIFY_THEN_TERMINATE"``, or + ``None``). + 2. It resolves the ``mode:`` expression against that. + 3. ``"TERMINATE"``/``"NOTIFY_THEN_TERMINATE"`` — the cancelation block + now acts as that method, and its sibling fields are validated + against that shape. ``None`` (null, whole-field expressions only) — + the whole ``cancelation:`` block is treated as never written. + Anything else — the action fails. + + Static validation is *not* deferred: at parse time the validator still + checks the expression is well-formed and that ``WrappedAction.*`` is + only referenced inside wrap hooks. Any format string is accepted — + normal interpolation like ``"{{Prefix}}_THEN_TERMINATE"`` is permitted; + only the resolved value is constrained. You just can't know *which* of + the two modes it'll be until the wrapped action is in front of you — + which is inherent to forwarding: the same wrap environment gets reused + across many steps whose cancelation settings differ. + + Mirrors ``CancelationMode::DeferredMode`` in openjd-rs. See + openjd-specifications Template Schemas 5.3 and RFC 0008 "Cancelation + behavior". + + Attributes: + mode (FormatString): A format string resolving to "TERMINATE" or + "NOTIFY_THEN_TERMINATE"; a whole-field interpolation expression + may also resolve to null. + notifyPeriodInSeconds (Optional[Union[int, FormatString]]): As on + CancelationMethodNotifyThenTerminate; only meaningful when the + mode resolves to NOTIFY_THEN_TERMINATE, and must resolve to + null when the mode resolves to TERMINATE. + """ + + mode: FormatString + notifyPeriodInSeconds: Optional[Union[NotifyPeriodType, FormatString]] = None # noqa: N815 + + _job_creation_metadata = JobCreationMetadata(resolve_fields={"notifyPeriodInSeconds"}) + + @field_validator("mode", mode="before") + @classmethod + def _validate_mode(cls, v: Any, info: ValidationInfo) -> Any: + if isinstance(v, str): + context = cast(Optional[ModelParsingContext], info.context) + if context and "FEATURE_BUNDLE_1" not in context.extensions: + raise ValueError( + "a format string in cancelation mode requires the FEATURE_BUNDLE_1 extension." + ) + # Any format string is permitted (normal format string + # behavior, Template Schemas 5.3); the resolved value is + # checked against the two mode names at run time. Only a + # whole-field expression additionally gets string? null + # semantics (a null result drops the cancelation object). + return v + + @field_validator("notifyPeriodInSeconds", mode="before") + @classmethod + def _validate_notify_period( + cls, v: Any, info: ValidationInfo + ) -> Optional[Union[int, FormatString]]: + return _validate_notify_period_value(v, info) + + +def _cancelation_discriminator(v: Any) -> Optional[str]: + """Callable discriminator for the cancelation union: routes the two + literal modes to their fixed-shape classes and a format-string mode to + :class:`CancelationMethodDeferred` (see that class's docstring for why + the mode decision can be deferred at all).""" + mode = v.get("mode") if isinstance(v, dict) else getattr(v, "mode", None) + if isinstance(mode, CancelationMode): + mode = mode.value + if isinstance(mode, str): + if mode == CancelationMode.NOTIFY_THEN_TERMINATE.value: + return "notify_then_terminate" + if mode == CancelationMode.TERMINATE.value: + return "terminate" + if "{{" in mode: + return "deferred" + if isinstance(v, CancelationMethodNotifyThenTerminate): + return "notify_then_terminate" + if isinstance(v, CancelationMethodTerminate): + return "terminate" + if isinstance(v, CancelationMethodDeferred): + return "deferred" + return None + + +CancelationMethod = Annotated[ + Union[ + Annotated[CancelationMethodNotifyThenTerminate, Tag("notify_then_terminate")], + Annotated[CancelationMethodTerminate, Tag("terminate")], + Annotated[CancelationMethodDeferred, Tag("deferred")], + ], + Discriminator(_cancelation_discriminator), +] + + ArgListType = Annotated[list[ArgString], Field(min_length=1)] # WRAP_ACTIONS (RFC 0008) wrap-hook field names on EnvironmentActions. @@ -436,17 +570,18 @@ class Action(OpenJDModel_v2023_09): timeout (Optional[int]): Maximum allowed runtime of the Action in seconds. Can be a format string with FEATURE_BUNDLE_1 extension. Default: No timeout - cancelation (Optional[Union[CancelationMethodNotifyThenTerminate, CancelationMethodTerminate]]): - If defined, provides details regarding how this action should be canceled. + cancelation (Optional[CancelationMethod]): If defined, provides details + regarding how this action should be canceled. One of + CancelationMethodNotifyThenTerminate, CancelationMethodTerminate, or + CancelationMethodDeferred (a format-string mode resolved + at run time; FEATURE_BUNDLE_1). Default: CancelationMethodTerminate """ command: CommandString args: Optional[ArgListType] = None timeout: Optional[Union[PositiveInt, FormatString]] = None - cancelation: Optional[ - Union[CancelationMethodNotifyThenTerminate, CancelationMethodTerminate] - ] = Field(None, discriminator="mode") + cancelation: Optional[CancelationMethod] = None _job_creation_metadata = JobCreationMetadata(resolve_fields={"timeout"}) @@ -786,9 +921,7 @@ class SimpleAction(OpenJDModel_v2023_09): script: DataString args: Optional[ArgListType] = None timeout: Optional[Union[PositiveInt, FormatString]] = None - cancelation: Optional[ - Union[CancelationMethodNotifyThenTerminate, CancelationMethodTerminate] - ] = Field(None, discriminator="mode") + cancelation: Optional[CancelationMethod] = None let: Optional[list[str]] = None # SimpleAction is syntax sugar that resolves to a StepScript (TASK scope), @@ -903,6 +1036,15 @@ class EnvironmentScript(OpenJDModel_v2023_09): "|WrappedAction.Args", "|WrappedAction.Environment", "|WrappedAction.Timeout", + # RFC 0008 follow-up (openjd-specifications#148): the wrapped + # action's cancelation config. Mode is string? — "TERMINATE", + # "NOTIFY_THEN_TERMINATE", or null when the wrapped action + # defines no . NotifyPeriodInSeconds is int? — the effective + # grace period for NOTIFY_THEN_TERMINATE (with the schema + # defaults applied: 120 for a task's onRun, 30 otherwise), and + # null when a notify period does not apply. + "|WrappedAction.Cancelation.Mode", + "|WrappedAction.Cancelation.NotifyPeriodInSeconds", "|WrappedEnv.Name", "|WrappedStep.Name", }, diff --git a/test/openjd/model_v0/format_strings/test_format_string.py b/test/openjd/model_v0/format_strings/test_format_string.py index a65b6994..0e3c45d3 100644 --- a/test/openjd/model_v0/format_strings/test_format_string.py +++ b/test/openjd/model_v0/format_strings/test_format_string.py @@ -108,6 +108,27 @@ def test_multiple_expressions( # THEN assert format_string.resolve(symtab=symtab) == expected + @pytest.mark.parametrize( + "input, expected", + [ + pytest.param("MODE=<{{Test.val}}>", "MODE=<>", id="none-only"), + pytest.param("{{Test.val}}", "", id="whole-string-none"), + ], + ) + def test_none_value_renders_as_empty(self, input: str, expected: str) -> None: + # A None value interpolates as the empty string, matching the EXPR + # engine's null rendering (RFC 0005). Relevant for the nullable + # WrappedAction.Cancelation.* injected symbols (RFC 0008 follow-up). + # GIVEN + symtab = SymbolTable() + + # WHEN + format_string = FormatString(input, context=ModelParsingContext_v2023_09()) + symtab["Test.val"] = None + + # THEN + assert format_string.resolve(symtab=symtab) == expected + def test_without_entry_in_table(self): # GIVEN input = " {{ Test.val }}-{{ Test.end}} " diff --git a/test/openjd/model_v0/format_strings/test_node.py b/test/openjd/model_v0/format_strings/test_node.py index adc34045..da0c8d51 100644 --- a/test/openjd/model_v0/format_strings/test_node.py +++ b/test/openjd/model_v0/format_strings/test_node.py @@ -39,3 +39,33 @@ def test_repr(self): # THEN assert str(node) == "FullName(Test.Name)" + + def test_evaluate_to_str_renders_none_as_empty(self): + # A None value interpolates as the empty string, matching the EXPR + # engine's null rendering (RFC 0005). Relevant for nullable injected + # symbols such as WrappedAction.Cancelation.Mode (string?) and + # WrappedAction.Cancelation.NotifyPeriodInSeconds (int?), which are + # None when the wrapped action defines no + # (RFC 0008 follow-up). + # GIVEN + symtab = SymbolTable() + symtab["Test.Name"] = None + node = FullNameNode("Test.Name") + + # WHEN + result = node.evaluate_to_str(symtab=symtab) + + # THEN + assert result == "" + + def test_evaluate_to_str_coerces_value_with_str(self): + # GIVEN + symtab = SymbolTable() + symtab["Test.Name"] = 45 + node = FullNameNode("Test.Name") + + # WHEN + result = node.evaluate_to_str(symtab=symtab) + + # THEN + assert result == "45" diff --git a/test/openjd/model_v0/v2023_09/test_wrap_actions.py b/test/openjd/model_v0/v2023_09/test_wrap_actions.py index f45ea4e4..90cc97b4 100644 --- a/test/openjd/model_v0/v2023_09/test_wrap_actions.py +++ b/test/openjd/model_v0/v2023_09/test_wrap_actions.py @@ -294,3 +294,108 @@ def test_create_job_with_step_env_wrap_succeeds(self): job = create_job(job_template=jt, job_parameter_values={}) step_env_actions = job.steps[0].stepEnvironments[0].script.actions assert step_env_actions.onWrapTaskRun is not None + + +class TestCancelationRoundTripForwarding: + """Cancelation round-trip forwarding (openjd-rs PR #261 review, + discussion r3597453516). A wrap hook must be able to forward the + wrapped action's cancelation verbatim via whole-field expressions:: + + cancelation: + mode: "{{WrappedAction.Cancelation.Mode}}" + notifyPeriodInSeconds: "{{WrappedAction.Cancelation.NotifyPeriodInSeconds}}" + + Under RFC 0005 type-forwarding, a single outer ``{{...}}`` forwards + the expression's type into the field: a string mode behaves like the + literal, and a null result means the field is omitted (a null mode + drops the whole cancelation object; a null period falls back to the + schema default). + + Mirrors ``cancelation_round_trip_*`` in openjd-rs + ``crates/openjd-model/tests/integration/test_wrap_actions.rs``. + + Note: the review example also forwards ``timeout:``; that is + deliberately out of scope pending the WrappedAction.Timeout + int-vs-int? question (the 0-when-unset sentinel is not a valid + ````). + """ + + _ROUND_TRIP_EXTS = ["WRAP_ACTIONS", "EXPR", "FEATURE_BUNDLE_1"] + + def _wrap_actions(self, cancelation: dict) -> dict: + actions = { + "onWrapEnvEnter": _cmd("{{WrappedAction.Command}}"), + "onWrapTaskRun": { + "command": "echo", + "args": ["{{WrappedAction.Command}}"], + "cancelation": cancelation, + }, + "onWrapEnvExit": _cmd("{{WrappedAction.Command}}"), + } + return actions + + def test_full_cancelation_forwarding_accepted(self): + # Mark's example from the PR #261 review thread (minus timeout): + # the whole-field expressions in the wrap hook's cancelation + # block must parse and validate. + tmpl = _env_template( + self._wrap_actions( + { + "mode": "{{WrappedAction.Cancelation.Mode}}", + "notifyPeriodInSeconds": ( + "{{WrappedAction.Cancelation.NotifyPeriodInSeconds}}" + ), + } + ), + extensions=self._ROUND_TRIP_EXTS, + ) + _decode(tmpl, extensions=self._ROUND_TRIP_EXTS) + + def test_notify_period_only_forwarding_accepted(self): + # The narrower forward: a literal mode with only the notify + # period forwarded. notifyPeriodInSeconds is already @fmtstring + # under FEATURE_BUNDLE_1; the whole-field expression is int? and + # a null result must drop the field (schema default applies). + tmpl = _env_template( + self._wrap_actions( + { + "mode": "NOTIFY_THEN_TERMINATE", + "notifyPeriodInSeconds": ( + "{{WrappedAction.Cancelation.NotifyPeriodInSeconds}}" + ), + } + ), + extensions=self._ROUND_TRIP_EXTS, + ) + _decode(tmpl, extensions=self._ROUND_TRIP_EXTS) + + def test_fmtstring_mode_requires_feature_bundle_1(self): + # The format-string mode form is gated on FEATURE_BUNDLE_1 + # (Template Schemas 5.3); with only WRAP_ACTIONS + EXPR it must + # be rejected. + tmpl = _env_template( + self._wrap_actions({"mode": "{{WrappedAction.Cancelation.Mode}}"}), + extensions=["WRAP_ACTIONS", "EXPR"], + ) + with pytest.raises(DecodeValidationError, match="FEATURE_BUNDLE_1"): + _decode(tmpl, extensions=["WRAP_ACTIONS", "EXPR"]) + + @pytest.mark.parametrize( + "mode", + [ + pytest.param("TERMIN{{WrappedAction.Cancelation.Mode}}", id="leading-text"), + pytest.param("{{ 'NOTIFY' }}_THEN_TERMINATE", id="trailing-text"), + ], + ) + def test_partial_fmtstring_mode_accepted(self, mode: str): + # A format-string mode gets normal format string behavior + # (Template Schemas 5.3): partial interpolation is statically + # valid, and the resolved value is checked against the two mode + # names at run time. Only a whole-field expression additionally + # gets string? null semantics (a null result drops the + # cancelation object). + tmpl = _env_template( + self._wrap_actions({"mode": mode}), + extensions=self._ROUND_TRIP_EXTS, + ) + _decode(tmpl, extensions=self._ROUND_TRIP_EXTS)