[codex] Harden E2E readiness and Dagster launch retries#63
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49c36af219
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
49c36af to
d50bc7d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9d1f86b46
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f4a69d076
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| existing = self.find_run_by_tag(launch_key) | ||
| if existing: | ||
| return existing |
There was a problem hiding this comment.
When a launch request times out or the connection drops after Dagster has accepted it but before runsOrError can see the new run, this single immediate lookup can return no matches and the loop then submits another launchRun with the same tag. The tag is only metadata on Dagster runs, so reusing it does not make the mutation idempotent; under a slow webserver this can launch the same product pipeline twice and duplicate/corrupt E2E data. Poll the tag for a bounded window, or use a true idempotency key, before retrying the mutation.
Useful? React with 👍 / 👎.
| if any(job in identity for job in suite_jobs): | ||
| return "suite-owned-job" |
There was a problem hiding this comment.
Because check_pods_ready runs before launch_and_poll_dagster_jobs, any failed Dagster Job pod from a previous sales_order_revenue_pipeline/product run is still present when this code inspects owner names and labels, so it matches PRODUCT_JOBS here and is classified as suite-owned-job. That leaves historical product-run pods as hard readiness blockers even though the current suite has not launched anything yet; classify by a run/launch tag from this invocation, or downgrade pre-existing Dagster run Jobs until the current run IDs are known.
Useful? React with 👍 / 👎.
1 participant
Summary
Implements #61 by making E2E readiness workload-aware and Dagster launch retries bounded and duplicate-safe.
Changes
Validation
uv run --project tools/olf ruff check tools/olfuv run --project tools/olf pytest tools/olf/tests/test_e2e.py(34 passed)make check-structure check-contracts check-infra check-project-code check-dbtgit diff --checkNo live local/AWS cluster was available for this PR; runtime validation remains an integration gate before merge.
Refs #61