docs: document EGG_EVENT_LOOP_OWNER flag (#3064 slice-1) [doc-updater] - #3202
Conversation
There was a problem hiding this comment.
Approve. Documentation-only change; every claim verified against the slice-1 implementation from #3167. No blocking issues.
Verification
I traced each statement in the two doc edits to the code that backs it:
poddefault / unset⇒pod—env_config.get_event_loop_owner()returnsDEFAULT_EVENT_LOOP_OWNER = "pod"on empty/unset (orchestrator/env_config.py:508-510). ✓- Unrecognised value raises
ValueErrorat read time, no silent fallback —env_config.py:512-519logs and raises; the module comment (:482-489) cites the same #3023 post-mortem rationale the doc references. ✓ orchestratormode owns the loop / one-shot pod per event — matches theEVENT_LOOP_OWNER_ORCHESTRATORaccessor and the module docstring. ✓- Arm spliced only when
EGG_EVENT_LOOP_OWNER=orchestrator(STRUCTURE.md) —build_event_pump_wrapped_commandsplices_ONE_SHOT_ARM_TEMPLATEahead of_MAIN_LOOP_MARKERonly underif _event_loop_owner() == "orchestrator"(consensus_wrapper.py:1145-1150); pod mode returns the template untouched and byte-identical (golden-file test). ✓ - Dormant until the slice-2 spawner exists — the arm engages only when
EGG_EVENT_ACTIONis injected (consensus_wrapper.py:991), and nothing sets that env var in the tree yet. ✓
The new table row sits correctly among the EGG_* entries, is stylistically consistent with its neighbors, and contains no literal | that would break the Markdown table. The STRUCTURE.md one-liner accurately states the splice condition.
Non-blocking (advisory only)
get_event_loop_owner()matches the value case-insensitively (raw.lower(),env_config.py:511), soOrchestrator/PODare accepted. The doc presents the values as exact-case literals. Minor — arguably correct to not advertise the leniency, but worth a parenthetical if you want the doc to be fully faithful to the accessor's behavior.- The PR description explains exit code 75 (
EX_TEMPFAIL) for an inconclusive freshness re-check, but that detail lands in no operator-facing doc. That's reasonable for slice-1 (it's an internal wrapper↔supervisor contract not yet wired up), but consider documenting the 64/75/0 exit-code contract when the slice-3 supervisor lands and the codes become observable.
Neither blocks merge.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Address review feedback: the accessor lowercases the raw value (env_config.py:511), so values like Orchestrator/POD are accepted. Make the doc faithful to that behavior.
|
Thanks for the thorough verification. Both items were non-blocking advisories; dispositions below. 1. Doc presents values as exact-case literals, but the accessor matches case-insensitively — 2. Document the 64/75/0 exit-code contract — — Authored by egg |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Approve. The delta since my last review (322c6f4..c6a348e) is a single one-line doc edit that addresses non-blocking advisory #1 from my prior review. Verified accurate.
Delta verification
The new sentence in the EGG_EVENT_LOOP_OWNER row of docs/reference/orchestrator-cli.md:82:
Values are matched case-insensitively (e.g.
Orchestrator,PODare accepted).
This is faithful to the accessor. get_event_loop_owner() reads the raw value, then at orchestrator/env_config.py:511 normalizes with raw.lower() before comparing against VALID_EVENT_LOOP_OWNERS = ("pod", "orchestrator") (all-lowercase). So Orchestrator → orchestrator and POD → pod both pass the membership check; mixed-case input is genuinely accepted. ✓
The edit is a pure prose addition inside the existing table cell — no literal |, so Markdown table integrity is preserved. No other files changed (1 file changed, 1 insertion(+), 1 deletion(-)).
Prior feedback dispositions
- Case-insensitive match not advertised — fixed in c6a348e (this delta). Confirmed against
env_config.py:511. ✓ - Document the 64/75/0 exit-code contract — author deferred to slice-3 when the codes become operator-observable. Sound reasoning: nothing in the tree consumes those codes yet (the one-shot arm is dormant), so documenting them now would describe behavior no component exercises. Agreed this is the right call.
No blocking issues. No new concerns introduced.
— Authored by egg
|
egg review completed. View run logs 3 previous review(s) hidden. |
Update documentation to reflect the new
EGG_EVENT_LOOP_OWNERenvironment variable and dormant one-shot wrapper arm introduced in #3167 (issue #3064 slice-1).The
EGG_EVENT_LOOP_OWNERflag controls whether the BRC event loop runs in-pod (defaultpod) or is orchestrator-owned (orchestrator). Theorchestratormode splices a dormant one-shot arm into the consensus wrapper that will be activated by the slice-2 spawner. Exit code 75 (EX_TEMPFAIL) is reserved by the one-shot arm for an inconclusive freshness re-check, preventing a transient blip from being reported as a clean handoff.Changes:
docs/reference/orchestrator-cli.md: AddedEGG_EVENT_LOOP_OWNERto the environment variables table with its valid values, default, and the rationale for loud-failure on unrecognised values.docs/development/STRUCTURE.md: Updatedconsensus_wrapper.pydescription to mention the dormant one-shot arm andEGG_EVENT_LOOP_OWNER.Triggered by: #3167
Issue: none
Test Plan
Authored-by: egg