Repository navigation
add_warning: store exceptions given as args as text, not the exception (1.4-maint) - #10571
Open
ThomasWaldmann wants to merge 1 commit into
Open
ThomasWaldmann wants to merge 1 commit into
ThomasWaldmann wants to merge 1 commit into
Conversation
For a BackupWarning, the args of the warning include the caught BackupError exception. Storing that in the global warnings list kept the exception - and thus its traceback and the frames (with all their locals) of the code that failed - alive for the rest of the borg run, one per warning: for a failed file in borg extract, that is the extract_item frame including the chunk data that was being written. On a full disk, every remaining file adds another chunk. Store the exception's message text instead, which is what formatting the warning message with the exception gives anyway. Add a regression test (the exception wrapped by a BackupWarning must be freed after print_warning_instance()) and a unit test for the get_ec() warnings logic. Backport of borgbackup#10331 (cherry picked from commit 366db25, tests moved to the 1.4-maint test modules). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1.4-maint #10571 +/- ##
=============================================
+ Coverage 82.11% 82.16% +0.04%
=============================================
Files 38 38
Lines 11485 11486 +1
Branches 1810 1810
=============================================
+ Hits 9431 9437 +6
+ Misses 1468 1465 -3
+ Partials 586 584 -2 ☔ View full report in Codecov by Harness. |
This branch has not been deployed
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.
Backport of #10331 to 1.4-maint.
What
For a
BackupWarning, the args of the warning include the caughtBackupErrorexception. Storing that in the global warnings list (add_warning()) kept the exception - and thus its traceback and the frames (with all their locals) of the code that failed - alive for the rest of the borg run, one per warning. For a failed file inborg extract, that is theextract_item()frame including the chunkdatathat was being written; on a full disk, every remaining file adds another chunk.How
add_warning()stores exceptions given as args as their message text (str(exc)), which is what formatting the warning message with the exception gives anyway. Thehelpers/__init__.pychange cherry-picked cleanly.Tests
The tests from #10331 are moved to the 1.4-maint test modules:
testsuite/archiver.py: regression test at theprint_warning_instance()level (the exception wrapped by theBackupWarningmust be freed afterwards, checked via weakref after agc.collect()for PyPy). Fails without the fix, passes with it.testsuite/helpers.py: unit test for theget_ec()warnings logic.🤖 Generated with Claude Code