Report one error per failed Globus transfer; hold restart for parked report - #127
Merged
Merged
Conversation
…the parked report is mailed; bump version to 3.0.19 pg_file.py: a cancelled transfer was logged twice - once as the full 17-line dsglobus get-task dump, once as the terse status line - so three failures read as six errors in the email. The cancel reason is now condensed to one line, cached in QCANCEL and appended to the single error that names the file. pg_cmd.py: init_dscheck restarted a record whose einfo still held a progress report, stranding it since the dscheck daemon skips running records. It now leaves the record for the daemon to mail first. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Failed Globus tasks without cached cancellation reasons can still exit silently.
Review effort: Balanced
Findings: 1
What changed in this PR
Improves Globus failure reporting and preserves parked dscheck reports before restarting jobs.
Changes:
- Condenses cancelled Globus task details into one error.
- Defers dscheck restart when an email report is pending.
- Bumps the package version to 3.0.19.
| File | Description |
|---|---|
src/rda_python_common/pg_file.py |
Caches and reports concise cancellation reasons. |
src/rda_python_common/pg_cmd.py |
Holds restart while a report is pending. |
src/rda_python_common/__init__.py |
Updates runtime version. |
README.md |
Updates documented version. |
pyproject.toml |
Updates package version. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+536
to
+540
| if task['stat'] == 'F' and not syserr: | ||
| # nothing waits on this task, so report the cancellation here instead | ||
| if task['id'] in self.QCANCEL: | ||
| self.errlog("{}: Cancel Task due to {}".format(task['id'], self.QCANCEL.pop(task['id'])), 'B', 1, logact) | ||
| break |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pg_file.py: a cancelled Globus transfer produced two error entries - the full 17-linedsglobus get-taskdump plus the terse status line - so 3 failures printed as 6 errors. The cancel reason is now condensed to one line (CONNECTION_RESET after 99.15GB of 15 file(s)), cached in the newQCANCELmap and appended to the single error that names the file.submit_globus_taskbreaks out on a failed task without logging and used to rely on that dump being the error; it now emits the compact cancel line itself so no failure goes silent.pg_cmd.py:init_dscheckrestarted a dscheck record whoseeinfostill held a parked progress report. The dscheck daemon skips running records, so the report was stranded until the next run overwrote it. The restart is now held back for one daemon pass.Test plan
ast.parseon both changed modulesdsquasar -A 4email: one error per failed transfer, reason still present