-
Notifications
You must be signed in to change notification settings - Fork 21
feat: Add WrappedAction.Cancelation.* wrap-hook template variables #313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switching Previously, a template with Since a bad mode string is a common authoring mistake, consider having the discriminator route unrecognized non-format-string values to one of the fixed-shape classes (e.g. |
||
|
|
||
|
|
||
| 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 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The RFC 0008 wrapped-variable scope check does not inspect Concretely, a template that puts |
||
|
|
||
| _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 <Cancelation>. 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", | ||
| }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CancelationMethodDeferreddeclaresresolve_fields={"notifyPeriodInSeconds"}, socreate_jobwill eagerly resolve this field against the job-creation symbol table. But the whole point of the deferred cancelation is forwarding, e.g.WrappedAction.*symbols are seeded only at session runtime (they are not in thecreate_jobsymbol table —_create_job.pypopulates it from job parameters only). Resolving{{WrappedAction.Cancelation.NotifyPeriodInSeconds}}there will hitFullNameNode.evaluate’s"... has no value"/ the EXPR undefined-symbol path and raise aFormatStringErrorout ofcreate_job.Note
Action.command/argsavoid this precisely because they are not inresolve_fields(which is whytest_create_job_with_wrap_env_succeedspasses with{{WrappedAction.Command}}in args). The forwarding round-trip tests (test_full_cancelation_forwarding_accepted,test_notify_period_only_forwarding_accepted) only call_decode, nevercreate_job, so this path is uncovered. Please add acreate_jobtest over a forwarded-cancelation wrap env; I suspectnotifyPeriodInSecondsmust be kept unresolved at job-creation time (likecommand/args) and resolved by the sessions runtime instead — mirroring the timeout-forwarding case the docstring already defers.