bc1bb/backup-script

Fork 1

fix(retry): remove always-False identity check in run_with_retry (S5727) #68

Open

Claude wants to merge 1 commit from Claude/backup-script:fix/issue-29-S5727-retry-identity-check into master

Claude commented

2026-05-18 01:29:51 +02:00

First-time contributor

Closes #29

Problem

SonarQube rule S5727 flags if last_exc is not None: in run_with_retry
(core/retry.py:126) as a constant comparison — last_exc is initialized to
None on line 121 and never reassigned inside the loop, so the identity
check is always False and the body is unreachable. The whole branch is dead
code, and the raise last_exc it gated would have raised None (the
adjacent S5632 finding, issue #30).

Fix

Delete the dead-code fallback entirely. With reraise=True on both
network_retrying() and kuma_retrying(), the tenacity iterator already
re-raises the final exception out of the with attempt: block, so the
post-loop code is reached only if the policy yielded zero attempts (a
misconfiguration). Keep that defensive case as an unconditional
raise RuntimeError(...) and document why the previous block was dead.

Verification

pytest -q tests/unit/ — all 211 tests pass, including the regression
suite in tests/unit/test_retry_policy.py which exercises the loop on
both retried and non-retried exception types.
mypy --strict src/gardien/core/ — clean (10 source files, no issues).

Closes #29 ### Problem SonarQube rule **S5727** flags `if last_exc is not None:` in `run_with_retry` (core/retry.py:126) as a constant comparison — `last_exc` is initialized to `None` on line 121 and never reassigned inside the loop, so the identity check is always False and the body is unreachable. The whole branch is dead code, and the `raise last_exc` it gated would have raised `None` (the adjacent S5632 finding, issue #30). ### Fix Delete the dead-code fallback entirely. With `reraise=True` on both `network_retrying()` and `kuma_retrying()`, the tenacity iterator already re-raises the final exception out of the `with attempt:` block, so the post-loop code is reached only if the policy yielded zero attempts (a misconfiguration). Keep that defensive case as an unconditional `raise RuntimeError(...)` and document why the previous block was dead. ### Verification - `pytest -q tests/unit/` — all 211 tests pass, including the regression suite in `tests/unit/test_retry_policy.py` which exercises the loop on both retried and non-retried exception types. - `mypy --strict src/gardien/core/` — clean (10 source files, no issues).

Claude added 1 commit

2026-05-18 01:29:51 +02:00

fix(retry): remove always-False identity check in run_with_retry (S5727) 732470818e

The `if last_exc is not None` check in run_with_retry was always False:
`last_exc` was initialized to None and never reassigned, so the branch was
dead code. Drop the dead branch and replace with an unconditional
RuntimeError for the (unreachable) zero-attempts case.

Refs #29

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u fix/issue-29-S5727-retry-identity-check:Claude-fix/issue-29-S5727-retry-identity-check

git switch Claude-fix/issue-29-S5727-retry-identity-check

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch master

git merge --no-ff Claude-fix/issue-29-S5727-retry-identity-check

git switch Claude-fix/issue-29-S5727-retry-identity-check

git rebase master

git switch master

git merge --ff-only Claude-fix/issue-29-S5727-retry-identity-check

git switch Claude-fix/issue-29-S5727-retry-identity-check

git rebase master

git switch master

git merge --no-ff Claude-fix/issue-29-S5727-retry-identity-check

git switch master

git merge --squash Claude-fix/issue-29-S5727-retry-identity-check

git switch master

git merge --ff-only Claude-fix/issue-29-S5727-retry-identity-check

git switch master

git merge Claude-fix/issue-29-S5727-retry-identity-check

git push origin master

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

bc1bb/backup-script!68

No description provided.

Rows
Columns