feat: RFC 0007/0008 — step-level let bindings, wrap-hook step names, template extensions#230
feat: RFC 0007/0008 — step-level let bindings, wrap-hook step names, template extensions#230leongdl wants to merge 2 commits into
Conversation
… names, template-declared extensions - Enter a step's environments with the step-level EXPR let bindings (RFC 0007) so environment variables and actions can reference them. The extra_let_bindings keyword is only forwarded to openjd-sessions when a step actually defines lets, so the CLI keeps working against older sessions releases for every template that doesn't use step-level lets. - Pass step_name to Session.run_task so RFC 0008 wrap hooks resolve WrappedStep.Name. - Enable only the extensions each template declares, and seed Job.Name into the symbol table. - Update test_local_session.py mock-call assertions for the environment-enter flow. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| session_id=self.session_id, | ||
| job_parameter_values=job_parameter_values, | ||
| # RFC 0007 §7.3.1 (EXPR): seeds the Job.Name template variable. | ||
| job_name=str(job.name), |
There was a problem hiding this comment.
Inconsistent backwards-compatibility handling for new openjd-sessions keywords.
This PR carefully guards extra_let_bindings — the comment at enter_environment (in _actions.py) states that older openjd-sessions releases do not accept that keyword, so it is only forwarded when step-level let bindings exist. But job_name here (and step_name in _actions.py:55, passed to run_task) is passed unconditionally, and pyproject.toml still pins openjd-sessions >= 0.10.7, < 0.11 (unchanged by this PR).
If the RFC-0007/0008 keywords (job_name, step_name) are not present in the minimum supported sessions version — which is exactly the premise behind guarding extra_let_bindings — then Session(...) and run_task(...) will raise TypeError: unexpected keyword argument for any user on that version, breaking every openjd run invocation regardless of whether the template uses these extensions.
Either raise the openjd-sessions lower bound to the first release that accepts job_name/step_name, or guard these the same way extra_let_bindings is guarded. Whichever is correct, the three new keywords should be handled consistently.
| # Extension behaviors activate only when declared by the templates being | ||
| # run (the job template and any external environment templates) — the | ||
| # CLI's --extensions list is what the CLI *accepts*, not what is enabled. | ||
| declared_extensions: list[str] = list(the_job.extensions or []) |
There was a problem hiding this comment.
Direct .extensions attribute access may raise AttributeError on older openjd-model.
the_job.extensions (line 554) and env_template.extensions (line 556) are new attribute accesses. pyproject.toml pins openjd-model >= 0.9, < 0.12 (unchanged by this PR). If the extensions attribute was introduced in a later model version than 0.9, these accesses raise AttributeError on the lower end of the supported range, breaking openjd run entirely.
Note the asymmetry with _session_manager.py:369, which defensively uses getattr(step, "let", None) for a similarly new model attribute. Consider using the same defensive pattern here (e.g. getattr(the_job, "extensions", None) or []), or raise the openjd-model floor to the first version that guarantees extensions on Job and EnvironmentTemplate.
Review fixes on top of the RFC 0007/0008 CLI commit: - Exceptions raised out of session actions (the RFC 0008 "at most one wrap environment" RuntimeError, expression ValueErrors) now surface as a clean error-status result instead of a raw traceback: guarded in run_environment_enters (routed through LocalSessionFailed) with a belt-and-suspenders catch in _run_local_session. - The enter-failure rollback only drops the environment from the CLI's entered list when the session did not register it — a post- registration raise (e.g. a failing `variables` expression) keeps the env tracked so cleanup exits it in LIFO order instead of masking the original error with "Must exit Environment X first". - Thread step_name into Session.enter_environment for step-environment enters so Step.Name (RFC 0007 §7.3.1) resolves in step-level `let` bindings and step-env actions — Rust CLI parity (openjd-rs run/mod.rs threads the step's resolved symtab). Capability-gated on the installed openjd-sessions accepting the kwarg. - run_step's redundant pass-kwarg-only-when-truthy branch collapsed (the compat guard lives in EnterEnvironmentAction.run); version-skew getattr documented with its removal condition. Tests: 294 passed under plain hatch and against the editable fixed model/sessions trees (new: Step.Name end-to-end template, step_name propagation, post-registration rollback, clean-error-result). ruff/black/mypy clean. Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>
| # popping it here would skip its onExit and desynchronize us | ||
| # from the session's LIFO exit ordering check, masking the | ||
| # original error with "Must exit Environment X first". | ||
| if env_id not in self._openjd_session.environments_entered: |
There was a problem hiding this comment.
Unguarded self._openjd_session.environments_entered in the new error path may itself raise AttributeError on older openjd-sessions.
This block is otherwise carefully defensive about version skew — step_name acceptance is probed with inspect.signature (_actions.py), and step.let is read via getattr(step, "let", None). But environments_entered is read bare on the Session. pyproject.toml still pins openjd-sessions >= 0.10.7, < 0.11 (unchanged by this PR). If this public property was introduced after 0.10.7, the access raises AttributeError here.
The consequence is worse than usual because this is inside the except (RuntimeError, ValueError) handler: an AttributeError here is not caught, so it escapes and masks the original enter failure with an unrelated traceback (and skips setting self.failed / raising LocalSessionFailed).
Please confirm environments_entered is present in the minimum pinned openjd-sessions (0.10.7). If it is guaranteed, this is fine; if not, guard it the same way as the neighboring session APIs (e.g. getattr(self._openjd_session, "environments_entered", ())) and raise the version floor.
1 participant
What was the problem/requirement? (What/Why)
The CLI needed the RFC 0007 (EXPR) / RFC 0008 (WRAP_ACTIONS) integration pieces so job templates using step-level
letbindings and wrap-hook environments run correctly throughopenjd run.What was the solution? (How)
One squashed commit on top of mainline:
letbindings (RFC 0007) so environment variables and actions can reference them. Backwards compatible: theextra_let_bindingskeyword is only forwarded to openjd-sessions when a step actually definesletbindings, so the CLI keeps working against older sessions releases for every template that doesn't use step-level lets (the default is simply "no extra bindings").step_nametoSession.run_taskso RFC 0008 wrap hooks resolveWrappedStep.Name.Job.Nameinto the symbol table.test_local_session.pymock-call assertions for the environment-enter flow.What is the impact of this change?
openjd runsupports RFC 0007/0008 templates end-to-end when paired with the RFC 0008 sessions branch, and continues to work with older sessions releases for templates that don't use the new features.How was this change tested?
hatch run test, macOS) with the RFC 0008 model/sessions branches installed (one intermittentTaskTimeouttiming flake under parallel load passes in isolation).Depends on: openjd-model-for-python
python-rsbranch and openjd-sessions-for-python #333. Merge last.Review fixes (second commit)
fix: Address review findings — clean errors, Step.Name threading, from a multi-aspect review with a post-fix clean sweep:RuntimeError, expressionValueErrors) now surface as an error-status result instead of a raw traceback.variablesexpression) keeps the env tracked so cleanup exits it in LIFO order instead of masking the original error.step_nameis threaded intoSession.enter_environmentfor step-environment enters soStep.Name(RFC 0007 §7.3.1) resolves in step-levelletbindings and step-env actions (openjd-rsrun/mod.rsthreads the step's resolved symtab). Capability-gated so the CLI keeps working against older openjd-sessions releases.getattrdocumented with its removal condition.Tests after fixes: 294 passed under plain hatch and against the editable fixed model/sessions trees (new: Step.Name end-to-end template test, post-registration rollback, clean-error-result). ruff/black/mypy clean.