Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

optional more specific return codes #7928

Closed
wants to merge 21 commits into from

Conversation

ThomasWaldmann
Copy link
Member

@ThomasWaldmann ThomasWaldmann commented Nov 9, 2023

default behaviour is mostly unchanged, but user can enable this via an environment variable.

fixes #6756 and some other issues.

@codecov-commenter
Copy link

codecov-commenter commented Nov 9, 2023

Codecov Report

Attention: 83 lines in your changes are missing coverage. Please review.

Comparison is base (e50d267) 82.33% compared to head (3a18393) 82.59%.
Report is 6 commits behind head on 1.2-maint.

Files Patch % Lines
src/borg/archiver.py 57.79% 46 Missing ⚠️
src/borg/remote.py 41.66% 10 Missing and 4 partials ⚠️
src/borg/helpers/checks.py 22.22% 7 Missing ⚠️
src/borg/helpers/errors.py 88.13% 5 Missing and 2 partials ⚠️
src/borg/helpers/__init__.py 90.47% 3 Missing and 3 partials ⚠️
src/borg/archive.py 83.33% 2 Missing ⚠️
src/borg/helpers/fs.py 75.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@              Coverage Diff              @@
##           1.2-maint    #7928      +/-   ##
=============================================
+ Coverage      82.33%   82.59%   +0.25%     
=============================================
  Files             38       38              
  Lines          10781    10926     +145     
  Branches        2064     2097      +33     
=============================================
+ Hits            8877     9024     +147     
+ Misses          1352     1342      -10     
- Partials         552      560       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…odes

If not set, it will default to "legacy" (always return 2 for errors).

This commit only changes the Error exception class and its subclasses.

The more specific exit codes need to be defined via .exit_mcode in the subclasses.
- also output modern rc and traceback yes/no
- recursive list of Error subclasses
- _export_tar: remove unneeded call to set_ec
  print_warning() already sets the exit code to EXIT_WARNING.

- msgpack version check: raise Error instead of calling set_ec
previously, this was handled in RPCError handler and always resulted in rc 2.

now re-raise Lock Exceptions locally, so it gives rc 2 (legacy) or 7x (modern).
Users using recently created repos (after borg 1.0.9) or
who followed the upgrade procedure of 1.0.9 or >1.2.4 will
never see this error msg.

So, have it as short as and similar to the ArchiveTAMRequiredError.
- implement updating exit code based on severity, including modern codes
- extend print_warning with kwargs wc (warning code) and wt (warning type)
- update a global warnings_list with warning_info elements
- create a class hierarchy below BorgWarning class similar to Error class
- diff: change harmless warnings about speed to rc == 0
- delete --force --force: change harmless warnings to rc == 0

Also:

- have BackupRaceConditionError as a more precise subclass of BackupError
also: use more union operators rather than .union()
this is not needed and getting rid of it makes
the code / behaviour simpler to understand:

if a fatal error is detected, we throw an exception.

if we encounter something warning worthy, we emit and collect the warning.

in a few cases, we directly call set_ec to set the
exit code as needed, e.g. if passing it through
from a subprocess.

also:
- get rid of Archiver.exit_code
- assert that return value of archiver methods is None
- fix a print_warning call to use the correct formatting method
stop directly accessing the variables from other modules.

prefix with underscore to indicate that these shall
only be used within this module and every other user
shall call the respective functions.
if we do multiple calls to Archiver.do_something(),
we need to reset the ec / warnings after each call,
otherwise they will keep growing (in severity, in length).
@ThomasWaldmann
Copy link
Member Author

superseded by #7976.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants