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 48.38% 49.64% +1.26%
==========================================
Files 40 40
Lines 3960 3970 +10
==========================================
+ Hits 1916 1971 +55
+ Misses 2044 1999 -45 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Warning Review limit reached
Next review available in: 59 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ 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
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