Skip to content

Commit 4e9a384

Browse files
committed
fix: Require openjd-sessions >= 0.10.11 and drop the version-skew guard
The declared floor could not satisfy the calls this package makes. `RunTaskAction` passes `step_name=` to `Session.run_task` unconditionally, but that keyword does not exist before openjd-sessions 0.10.11 -- so against the old `>= 0.10.7` floor every task run would raise `TypeError: run_task() got an unexpected keyword argument 'step_name'`. 0.10.11 is released and is the first version with `step_name` on **both** `run_task` and `enter_environment`, so the floor moves there and the feature-detection guard goes away. That guard was doing real harm rather than providing real compatibility: - `_ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME` gated only `enter_environment`, while `run_task` was already unguarded. So the two call sites disagreed about which sessions versions this package supported, and the stricter of the two was the ungated one. - **It silenced the tests that prove RFC 0007 Step.Name works.** `test_localsession_step_env_enter_receives_step_name` branched on the flag and, when it was false, asserted the keyword was *absent* -- passing while proving the opposite of its name. Worse, `test_do_run_step_name_in_step_environment`, the only end-to-end test that actually resolves `Step.Name` inside a step environment, was `skipif`-ed away entirely. In a dev environment with a sessions build predating the keyword, both reported success while the feature was untested. Both are now unconditional, and both fail if either call site stops forwarding the name: 2 mutants, 0 survivors (`EnterEnvironmentAction` and `RunTaskAction` each stopping forwarding `step_name`). `extra_let_bindings` keeps its `if self._extra_let_bindings:` check, and `step_name` keeps its `is not None` check, but neither is feature-detection now -- they are "only forward what carries something". Job and external environment enters have no owning step, so `Step.Name` must stay undefined for them rather than be seeded with None, which the existing assertions pin. Release ordering note: openjd-sessions 0.10.11 raises when a wrap environment is active and no `step_name` is given, so the released CLI cannot run wrapped tasks until this lands. That makes this the unblocking change rather than a cleanup. Verified: 295 passed, 2 skipped (both Windows-shell tests); ruff, black, and mypy clean. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
1 parent daed283 commit 4e9a384

4 files changed

Lines changed: 26 additions & 36 deletions

File tree

pyproject.toml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ classifiers = [
2929
"Intended Audience :: End Users/Desktop"
3030
]
3131
dependencies = [
32-
"openjd-sessions >= 0.10.7,< 0.11",
32+
# 0.10.11 is the first release with `step_name` on both Session.run_task and
33+
# Session.enter_environment (RFC 0007/0008). Below it, run_task() raises
34+
# TypeError on every task run, since this CLI passes the keyword unconditionally.
35+
"openjd-sessions >= 0.10.11,< 0.11",
3336
"openjd-model >= 0.9,< 0.12"
3437
]
3538

src/openjd/cli/_run/_local_session/_actions.py

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,12 @@
11
# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
22

3-
import inspect
43
from enum import Enum
54
from typing import Any, Optional
65

76
from openjd.model import Step, TaskParameterSet
87
from openjd.model.v2023_09 import Environment
98
from openjd.sessions import Session
109

11-
# Version-skew guard: the step_name keyword was added to
12-
# Session.enter_environment alongside RFC 0007 Step.Name support. Detect it
13-
# once so this CLI keeps working against older openjd-sessions releases that
14-
# don't accept the keyword (every step-environment enter carries a step name,
15-
# so a value-presence check alone isn't enough here).
16-
_ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME: bool = (
17-
"step_name" in inspect.signature(Session.enter_environment).parameters
18-
)
19-
2010

2111
class EnvironmentType(str, Enum):
2212
"""
@@ -97,20 +87,16 @@ def __init__(
9787
self._step_name = step_name
9888

9989
def run(self):
100-
# Backwards compatibility: only forward `extra_let_bindings` when the
101-
# step actually defines `let` bindings (RFC 0007). Older
102-
# openjd-sessions releases don't accept the keyword, so omitting it
103-
# by default keeps this CLI working with any sessions version for
104-
# every template that doesn't use step-level lets — the reasonable
105-
# default is simply "no extra bindings". `step_name` follows the same
106-
# pattern, but since every step-environment enter has a step name, it
107-
# is additionally gated on the installed openjd-sessions accepting
108-
# the keyword; without it, Step.Name simply stays undefined (the
109-
# pre-RFC 0007 behavior).
90+
# Both keywords are guaranteed by this package's `openjd-sessions`
91+
# floor (>= 0.10.11), so neither is feature-detected. They are still
92+
# only forwarded when they carry something: a step with no `let`
93+
# bindings means "no extra bindings", and job/external environment
94+
# enters have no owning step, so `Step.Name` must stay undefined for
95+
# them rather than being seeded with None.
11096
optional_kwargs: dict[str, Any] = {}
11197
if self._extra_let_bindings:
11298
optional_kwargs["extra_let_bindings"] = self._extra_let_bindings
113-
if self._step_name is not None and _ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME:
99+
if self._step_name is not None:
114100
optional_kwargs["step_name"] = self._step_name
115101
self._session.enter_environment(
116102
environment=self._environment,

test/openjd/cli/test_local_session.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
from . import SampleSteps, SESSION_PARAMETERS
88
from openjd.model import StepParameterSpaceIterator
99
from openjd.sessions import Session, SessionState
10-
from openjd.cli._run._local_session._actions import _ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME
1110
from openjd.cli._run._local_session._session_manager import (
1211
LocalSession,
1312
EnvironmentType,
@@ -207,8 +206,14 @@ def test_localsession_step_env_enter_receives_step_name(
207206
):
208207
"""
209208
RFC 0007 §7.3.1 (EXPR): a step-environment enter passes the owning step's
210-
name to Session.enter_environment (when the installed openjd-sessions
211-
accepts the keyword), while job/external environment enters never do.
209+
name to Session.enter_environment, while job/external environment enters
210+
never do.
211+
212+
The step-environment assertion used to be gated on feature-detecting the
213+
keyword, which meant that against a sessions build without it the test took
214+
the other branch and asserted the keyword was *absent* -- passing while
215+
proving the opposite of its name. The `openjd-sessions >= 0.10.11` floor
216+
guarantees the keyword, so the assertion is now unconditional.
212217
"""
213218
sample_job, sample_job_parameters, template_dir, current_working_dir = sample_job_and_dirs
214219
patched_enter = patched_actions[0]
@@ -233,12 +238,7 @@ def test_localsession_step_env_enter_receives_step_name(
233238

234239
assert step_env_calls
235240
for enter_call in step_env_calls:
236-
if _ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME:
237-
assert enter_call.kwargs["step_name"] == sample_job.steps[SampleSteps.NormalStep].name
238-
else:
239-
# Older openjd-sessions releases don't accept the keyword; the
240-
# version-skew guard must omit it.
241-
assert "step_name" not in enter_call.kwargs
241+
assert enter_call.kwargs["step_name"] == sample_job.steps[SampleSteps.NormalStep].name
242242

243243
# Job/external environment enters never carry a step name.
244244
assert other_env_calls

test/openjd/cli/test_run_command.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
_process_task_params,
2121
_process_tasks,
2222
)
23-
from openjd.cli._run._local_session._actions import _ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME
2423
from openjd.cli._run._local_session._session_manager import LoggingTimestampFormat
2524
from openjd.sessions import LOG as SessionsLogger, PathMappingRule, PathFormat, Session
2625

@@ -674,15 +673,17 @@ def test_run_local_session_enter_environment_raises(capsys: pytest.CaptureFixtur
674673
assert "Traceback" not in outerr.out + outerr.err
675674

676675

677-
@pytest.mark.skipif(
678-
not _ENTER_ENVIRONMENT_ACCEPTS_STEP_NAME,
679-
reason="Installed openjd-sessions does not accept step_name on enter_environment",
680-
)
681676
def test_do_run_step_name_in_step_environment(capsys: pytest.CaptureFixture) -> None:
682677
"""
683678
RFC 0007 §7.3.1 (EXPR) parity with openjd-rs: a step-level `let` binding
684679
may reference Step.Name, and the step's environments are entered with the
685680
binding so their actions can echo it.
681+
682+
This is the end-to-end proof of the feature. It used to be `skipif`-gated on
683+
feature-detecting the `step_name` keyword, which meant it did not run at all
684+
against a sessions build that lacked it -- so the only test that actually
685+
exercised Step.Name in a step environment was silently skipped. The
686+
`openjd-sessions >= 0.10.11` floor guarantees the keyword, so it always runs.
686687
"""
687688
template_dir = Path(__file__).parent / "templates"
688689
args = [

0 commit comments

Comments
 (0)