Skip to content

14.0 fix queue job cron jobrunner - #643

Merged
OCA-git-bot merged 1 commit into
OCA:14.0from
petrus-v:14.0-fix-queue_job_cron_jobrunner
Sep 12, 2024
Merged

14.0 fix queue job cron jobrunner#643
OCA-git-bot merged 1 commit into
OCA:14.0from
petrus-v:14.0-fix-queue_job_cron_jobrunner

Conversation

@petrus-v

@petrus-vpetrus-v commented Apr 10, 2024

Copy link
Copy Markdown
Contributor

No description provided.

@petrus-v
petrus-v changed the base branch from 16.0 to 14.0April 10, 2024 16:30
@petrus-v
[FIX] queue_job_cron_jobrunner: use priority to select job
148fb90
* use FIFO, firt createad job will be treat first
* if priority are different it take the precedent
Yet we are not using channel priority into account
@petrus-v
petrus-vforce-pushed the 14.0-fix-queue_job_cron_jobrunner branch from 158ff89 to 148fb90CompareApril 10, 2024 16:38
alexandregaldeano
alexandregaldeano approved these changes Apr 12, 2024
View reviewed changes

@alexandregaldeanoalexandregaldeano 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.

LGTM

sbidoul
sbidoul approved these changes Apr 14, 2024
View reviewed changes

@sbidoulsbidoul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given the documentation of the ChannelJob class, this fix looks correct

 Channel jobs are comparable according to the following rules:
* jobs with an eta come before all other jobs
* then jobs with a smaller eta come first
* then jobs with a smaller priority come first
* then jobs with a smaller creation time come first
* then jobs with a smaller sequence come first

@sbidoul

sbidoul commented Apr 14, 2024

Copy link
Copy Markdown
Member

cc/ @simahawk

@OCA-git-bot

OCA-git-bot commented Apr 15, 2024

Copy link
Copy Markdown
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@dreispt
dreispt requested a review from simahawkMay 11, 2024 16:54
@dreispt

dreispt commented May 11, 2024

Copy link
Copy Markdown
Member

@simahawk care to review?

@petrus-v

petrus-v commented Sep 12, 2024

Copy link
Copy Markdown
ContributorAuthor

Hi @ivantodorovich ! I hope this finds you well :)

As you are the maintainer of this module do you mind to review and merge it if you're fin with it ?

simahawk
simahawk approved these changes Sep 12, 2024
View reviewed changes

@simahawksimahawk 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.

LGTM, thanks!

@sbidoul

sbidoul commented Sep 12, 2024

Copy link
Copy Markdown
Member

/ocabot merge patch

@OCA-git-bot

OCA-git-bot commented Sep 12, 2024

Copy link
Copy Markdown
Contributor

On my way to merge this fine PR!
Prepared branch 14.0-ocabot-merge-pr-643-by-sbidoul-bump-patch, awaiting test results.

@OCA-git-bot
OCA-git-bot merged commit 972727d into OCA:14.0Sep 12, 2024
@OCA-git-bot

OCA-git-bot commented Sep 12, 2024

Copy link
Copy Markdown
Contributor

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

This was referenced Dec 27, 2024
Closed
Merged
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@simahawksimahawksimahawk approved these changes
@sbidoulsbidoulsbidoul approved these changes
+1 more reviewer
@alexandregaldeanoalexandregaldeanoalexandregaldeano approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approvedmerged 🎉ready to merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@petrus-v@sbidoul@OCA-git-bot@dreispt@simahawk@alexandregaldeano