refactor: simplify GHCR deploy poller - #136
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce4e9a68a3
ℹ️ 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".
| rollback_once( | ||
| compose_file=compose_file, | ||
| deploy_env_path=deploy_env_path, | ||
| previous_image_ref=current_image_ref, | ||
| command_runner=command_runner, |
There was a problem hiding this comment.
Skip rollback when prior env digest is not deployable
run_once now always rolls back to the pre-run COMIC_CRAWLER_IMAGE_REF after any deploy failure, but this ref can be an initial placeholder (the docs allow sha256:replace-me before first deploy). In that case, a failed first deploy immediately triggers a rollback to an invalid digest, which reliably fails and obscures the real failure path instead of falling back to manual intervention. Rollback should be conditional on having a known-good previous digest/reference.
Useful? React with 👍 / 👎.
| rollback_once( | ||
| compose_file=compose_file, | ||
| deploy_env_path=deploy_env_path, | ||
| previous_image_ref=current_image_ref, | ||
| command_runner=command_runner, | ||
| redaction_secrets=redaction_secrets, | ||
| ) |
There was a problem hiding this comment.
Preserve original deploy error when rollback fails
The rollback call in run_once is no longer wrapped in its own try/except, so if rollback fails (for example, old image pull/up fails), the function raises only the rollback exception and drops the original deploy/smoke-check error context. This regresses troubleshooting and alert quality compared to the prior behavior that reported both failure stages. Catch rollback exceptions and re-raise with both causes (and a rollback-failed notification) to keep incident diagnosis actionable.
Useful? React with 👍 / 👎.
Summary
Test Plan