Stop doubling the trailing bang in email headers; restore Globus error limit - #128
Merged
Merged
Conversation
…us error limit; bump version to 3.0.20 pg_log.py: set_email appended its own '!' to an EMLTOP message that already ended with one, giving the '...0 tarred!!' seen in dsquasar reports. It now adds the bang only when the caller left it off, and drops the caller's bang before continuing the sentence with ' with N Errors:'. pg_file.py: ELMTS['B'] back to 5. A failed Globus transfer used to be logged twice, so the limit of 10 gave up after 5 failures; 3.0.19 made it one error per failure, which had silently doubled the tolerance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Newline-terminated error headers and the legacy email API remain unfixed, while some Globus failures still count twice against the lower limit.
Review effort: Balanced
Findings: 3
Open (3)
What changed in this PR
This PR updates email-header formatting and restores the intended Globus failure limit for a 3.0.20 release.
Changes:
- Avoid adding a second
!to class-based email headers. - Lower the Globus error limit from 10 to 5.
- Update the package version and README example.
| File | Description |
|---|---|
src/rda_python_common/pg_log.py |
Changes top-level email-header formatting. |
src/rda_python_common/pg_file.py |
Lowers the Globus error limit. |
src/rda_python_common/__init__.py |
Updates the package version. |
README.md |
Updates the documented version. |
pyproject.toml |
Updates the distribution version. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| self.ELMTS = {'D': 20, 'H': 20, 'L': 20, 'R': 20, 'O': 10, 'B': 10} | ||
| # 'B' is 5 because a failed Globus transfer counts once; it used to be logged | ||
| # twice per failure, so the old limit of 10 also gave up after 5 failures | ||
| self.ELMTS = {'D': 20, 'H': 20, 'L': 20, 'R': 20, 'O': 10, 'B': 5} |
Comment on lines
+255
to
+256
| if not msg.endswith('\n'): | ||
| msg += "\n" if msg.endswith('!') else "!\n" |
| if not msg.endswith('\n'): | ||
| msg += "\n" if msg.endswith('!') else "!\n" | ||
| else: | ||
| if msg.endswith('!'): msg = msg[:-1] |
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_log.py:set_emailappended its own!to anEMLTOPmessage that already ended with one, producing...0 tarred!!in dsquasar progress reports. The bang is now added only when the caller left it off, and the caller's bang is dropped before the sentence continues withwith N Errors:(so the header reads...of All datasets with 6 Errors:rather than...datasets! with 6 Errors:).pg_file.py:ELMTS['B']back to 5. A failed Globus transfer used to be logged twice, so the limit of 10 gave up after 5 failures; 3.0.19 made it one error per failure, which silently doubled the tolerance.Test plan
ast.parseclean on both modules!