Skip to content

[17.0][FIX] queue:job: Handle zero max_retries in job retry logic - #816

Merged
OCA-git-bot merged 1 commit into
OCA:17.0from
heligrafics:fix/dead-jobs-max_retries
Aug 20, 2025
Merged

[17.0][FIX] queue:job: Handle zero max_retries in job retry logic#816
OCA-git-bot merged 1 commit into
OCA:17.0from
heligrafics:fix/dead-jobs-max_retries

Conversation

@zamberjo

@zamberjozamberjo commented Aug 18, 2025

Copy link
Copy Markdown
Member

Resolves#814

@OCA-git-bot

OCA-git-bot commented Aug 18, 2025

Copy link
Copy Markdown
Contributor

Hi @guewen,
some modules you are maintaining are being modified, check this out!

@sersanchus

sersanchus commented Aug 18, 2025

Copy link
Copy Markdown

TSGTM. I have verified that these changes ensure the job will be re-enqueued indefinitely when max_retries is set to 0 and the worker dies unexpectedly (e.g., abrupt termination).

@zamberjo
zamberjo marked this pull request as ready for review August 18, 2025 17:04
CopilotAI review requested due to automatic review settings August 18, 2025 17:04
CopilotAI reviewed Aug 18, 2025
View reviewed changes

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Fixes handling of zero max_retries in job retry logic by treating zero as NULL to prevent premature job failure. The issue occurred when jobs with max_retries=0 were incorrectly marked as failed instead of being requeued.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment threadqueue_job/jobrunner/runner.py Outdated
@zamberjozamberjo mentioned this pull request Aug 19, 2025
Merged
sbidoul
sbidoul reviewed Aug 19, 2025
View reviewed changes
Comment threadqueue_job/jobrunner/runner.py Outdated
@zamberjo
zamberjoforce-pushed the fix/dead-jobs-max_retries branch from fb1f1c4 to fa39043CompareAugust 19, 2025 08:44
@zamberjo
zamberjo requested a review from sbidoulAugust 19, 2025 08:45
AnizR
AnizR approved these changes Aug 19, 2025
View reviewed changes

@AnizRAnizR left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM, thanks for your contribution!

amh-mw
amh-mw reviewed Aug 19, 2025
View reviewed changes
Comment threadqueue_job/jobrunner/runner.py Outdated
@zamberjo
zamberjoforce-pushed the fix/dead-jobs-max_retries branch from fa39043 to 97f20e2CompareAugust 19, 2025 14:48
@zamberjo
zamberjo requested a review from amh-mwAugust 19, 2025 14:48
@zamberjo

zamberjo commented Aug 19, 2025

Copy link
Copy Markdown
MemberAuthor

@OCA/queue-maintainers Can this be merged?

@sbidoul

sbidoul commented Aug 20, 2025

Copy link
Copy Markdown
Member

/ocabot merge patch

@OCA-git-bot

OCA-git-bot commented Aug 20, 2025

Copy link
Copy Markdown
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 17.0-ocabot-merge-pr-816-by-sbidoul-bump-patch, awaiting test results.

@OCA-git-bot
OCA-git-bot merged commit a7a4529 into OCA:17.0Aug 20, 2025
7 checks passed
@OCA-git-bot

OCA-git-bot commented Aug 20, 2025

Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at 357fbd0. Thanks a lot for contributing to OCA. ❤️

@zamberjo
zamberjo deleted the fix/dead-jobs-max_retries branch August 20, 2025 07:45
@sbidoulsbidoul mentioned this pull request Aug 20, 2025
Merged
@sbidoul

sbidoul commented Aug 20, 2025•
edited
Loading

Copy link
Copy Markdown
Member

Backport to 16.0 in #817

@sbidoul

sbidoul commented Aug 20, 2025

Copy link
Copy Markdown
Member

And cherry-picked into #749 for 18.0

@simahawksimahawk mentioned this pull request Aug 20, 2025
Merged
@AnizRAnizR mentioned this pull request Sep 15, 2025
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code reviewCopilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@sbidoulsbidoulsbidoul approved these changes
+2 more reviewers
@amh-mwamh-mwamh-mw approved these changes
@AnizRAnizRAnizR approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approvedmerged 🎉

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@zamberjo@OCA-git-bot@sersanchus@sbidoul@amh-mw@AnizR