fix: preserve cloud tickets and per-job log offsets - #623
Conversation
Preserve caller-provided Bohrium tickets when the environment does not supply a non-empty override, and keep log download offsets independently for each job. Coding-Agent: Codex Codex-Version: codex-cli 0.144.4 Model: gpt-5.6-sol Reasoning-Effort: xhigh
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #623 +/- ##
==========================================
+ Coverage 57.51% 58.66% +1.14%
==========================================
Files 40 40
Lines 4256 4265 +9
==========================================
+ Hits 2448 2502 +54
+ Misses 1808 1763 -45 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Warning Review limit reachedNext included review available in 7 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe cloud client centralizes ticket selection, avoids unnecessary login when a ticket is available, and tracks log download offsets independently for each job. New tests cover ticket precedence, password login, request headers, and multi-job log polling. ChangesCloud client behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dpdispatcher/utils/dpcloudserver/client.py`:
- Around line 281-286: Update the requests.get call in the log polling method to
pass the established HTTP_TIME_OUT value and validate the response before using
resp.content. Raise or otherwise handle unsuccessful responses so error payloads
are not returned or counted in self.last_log_offsets, and update
request_get.call_args_list test assertions to include the timeout argument.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a0b5cf4d-4e38-451d-9957-39ed3a880e46
📒 Files selected for processing (2)
dpdispatcher/utils/dpcloudserver/client.pytests/test_dpcloudserver_client.py
njzjz-bot
left a comment
There was a problem hiding this comment.
The ticket preservation and per-job offset split are sound, but one unresolved log-fetch reliability issue remains: failed or unbounded HTTP requests can hang polling and corrupt the new per-job offset. I independently confirmed the existing unresolved thread; the actionable fix is inline.
This PR is authored by the reviewing account, so this is a comment review rather than a self-requested change.
Review context: Codex's quota is close to resetting, so I'm deliberately spending the remaining tokens on reviewing the open PR queue now.
Coding agent: Codex
Codex version: codex-cli 0.144.6
Model: gpt-5.6-sol
Reasoning effort: xhigh
Add timeouts and preserve per-job offsets when remote log requests fail. Coding-Agent: Codex Codex-Version: codex-cli 0.144.6 Model: gpt-5.6-sol Reasoning-Effort: xhigh
njzjz-bot
left a comment
There was a problem hiding this comment.
LGTM. The updated cloud log download now bounds requests, handles request failures, rejects unsuccessful responses, and advances offsets only after a successful fetch. The previous finding is resolved.
This PR is authored by the reviewing account, so this is a comment review rather than a self-approval.
Coding agent: Codex
Codex version: codex-cli 0.149.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
njzjz-bot
left a comment
There was a problem hiding this comment.
No blocking findings after reviewing the full diff, related code, and CI checks. Intended decision: APPROVE. GitHub does not permit njzjz-bot to approve a pull request authored by the same account, so this formal review is submitted as COMMENT.
Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
njzjz-bot
left a comment
There was a problem hiding this comment.
Reviewed the complete diff and relevant surrounding code. No blocking findings. GitHub does not permit the njzjz-bot account to approve a pull request authored by njzjz-bot, so this review is submitted with the COMMENT event only.
Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
njzjz-bot
left a comment
There was a problem hiding this comment.
Independent review result: no blocking issues found. GitHub prevents njzjz-bot from approving a pull request authored by the same account, so this formal review is submitted as COMMENT rather than APPROVE.
Coding agent: Codex
Codex version: codex-cli 0.151.0
Model: gpt-5.6-sol
Reasoning effort: xhigh
Resolve the conflicts with the latest master while preserving explicit cloud tickets and per-job log offsets. Coding-Agent: Codex Codex-Version: codex-cli 0.151.0 Model: gpt-5.6-sol Reasoning-Effort: xhigh
Fixes #614
Summary
Client(ticket=...)whenBOHR_TICKETis unset or emptyValidation
python -m coverage run -p --source=./dpdispatcher -m unittest(169 passed, 42 skipped)python -m coverage combine && python -m coverage report(49% total)pre-commit run --all-filesuvx ty check --python /tmp/dpdispatcher-validation-venv/bin/python --python-version 3.13dpdisp --helppython -m dpdispatcher -hmake htmlindoc/(succeeded with existing warnings)pyrightwas attempted, but its local wrapper could not provision a Node runtime; the repository's currenttycheck passesCoding agent: Codex
Codex version: codex-cli 0.144.4
Model: gpt-5.6-sol
Reasoning effort: xhigh
Summary by CodeRabbit
Bug Fixes
Tests