Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/openjd/model/_format_strings/_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/openjd/model/v2023_09/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
AttributeCapabilityValue,
AttributeRequirement,
AttributeRequirementTemplate,
CancelationMethodDeferred,
CancelationMethodNotifyThenTerminate,
CancelationMethodTerminate,
CancelationMode,
Expand Down Expand Up @@ -113,6 +114,7 @@
"AttributeCapabilityValue",
"AttributeRequirement",
"AttributeRequirementTemplate",
"CancelationMethodDeferred",
"CancelationMethodNotifyThenTerminate",
"CancelationMethodTerminate",
"CancelationMode",
Expand Down
194 changes: 168 additions & 26 deletions src/openjd/model/v2023_09/_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,14 @@
field_validator,
model_validator,
ConfigDict,
Discriminator,
StringConstraints,
Field,
PositiveInt,
PositiveFloat,
StrictBool,
StrictInt,
Tag,
ValidationError,
ValidationInfo,
)
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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):
Expand All @@ -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"})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CancelationMethodDeferred declares resolve_fields={"notifyPeriodInSeconds"}, so create_job will eagerly resolve this field against the job-creation symbol table. But the whole point of the deferred cancelation is forwarding, e.g.

cancelation:
  mode: "{{WrappedAction.Cancelation.Mode}}"
  notifyPeriodInSeconds: "{{WrappedAction.Cancelation.NotifyPeriodInSeconds}}"

WrappedAction.* symbols are seeded only at session runtime (they are not in the create_job symbol table — _create_job.py populates it from job parameters only). Resolving {{WrappedAction.Cancelation.NotifyPeriodInSeconds}} there will hit FullNameNode.evaluate’s "... has no value" / the EXPR undefined-symbol path and raise a FormatStringError out of create_job.

Note Action.command/args avoid this precisely because they are not in resolve_fields (which is why test_create_job_with_wrap_env_succeeds passes with {{WrappedAction.Command}} in args). The forwarding round-trip tests (test_full_cancelation_forwarding_accepted, test_notify_period_only_forwarding_accepted) only call _decode, never create_job, so this path is uncovered. Please add a create_job test over a forwarded-cancelation wrap env; I suspect notifyPeriodInSeconds must be kept unresolved at job-creation time (like command/args) and resolved by the sessions runtime instead — mirroring the timeout-forwarding case the docstring already defers.


@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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switching cancelation from pydantic's native Field(..., discriminator="mode") (a Literal-tagged union) to this callable Discriminator degrades the error message for an invalid literal mode.

Previously, a template with cancelation: {mode: "TERMINATEX"} (or any typo / wrong-case value) produced a clear, actionable error like Input should be 'NOTIFY_THEN_TERMINATE' or 'TERMINATE'. Now _cancelation_discriminator falls through all branches and returns None for such a value, so pydantic raises the generic union_tag_invalid error (Unable to extract tag using discriminator ...), which does not tell the author what the valid modes are.

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. terminate) so the field validator emits the specific enum error, or otherwise surface the allowed mode names in the failure.



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.
Expand Down Expand Up @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The RFC 0008 wrapped-variable scope check does not inspect cancelation. _action_referenced_namespaces (lines 544-548) only walks command, args, and timeout, so WrappedAction.* / WrappedEnv.* / WrappedStep.* referenced inside cancelation.mode or cancelation.notifyPeriodInSeconds are invisible to _validate_wrapped_variable_scope.

Concretely, a template that puts cancelation: {mode: "{{WrappedAction.Cancelation.Mode}}"} on an ordinary onEnter/onExit action (not a wrap hook) will be accepted, contradicting CancelationMethodDeferred's own docstring: "at parse time the validator still checks ... that WrappedAction.* is only referenced inside wrap hooks." The new deferred cancelation is precisely what makes such references possible in this field, so the scope enforcement should be extended to cover the cancelation sub-model's FormatString fields.


_job_creation_metadata = JobCreationMetadata(resolve_fields={"timeout"})

Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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",
},
Expand Down
21 changes: 21 additions & 0 deletions test/openjd/model_v0/format_strings/test_format_string.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}} "
Expand Down
30 changes: 30 additions & 0 deletions test/openjd/model_v0/format_strings/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 <Cancelation>
# (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"
Loading
Loading