Skip to content

[17.0] [IMP] queue_job: add_depends on RetryableJobError #786

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

Open
wants to merge 1 commit into
base: 17.0
Choose a base branch
from

Conversation

ivantodorovich
Copy link
Contributor

This commit adds the posibility to add new dependencies to a job when raising a RetryableJobError.

This commit adds the posibility to add new dependencies to a job when
raising a RetryableJobError.
@OCA-git-bot
Copy link
Contributor

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

@ivantodorovich ivantodorovich marked this pull request as ready for review June 6, 2025 15:59
Copy link
Contributor

@SilvioC2C SilvioC2C left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@@ -576,6 +576,11 @@ def perform(self):
try:
self.result = self.func(*tuple(self.args), **self.kwargs)
except RetryableJobError as err:
if err.add_depends:
from .delay import DelayableGraph
Copy link
Contributor

Choose a reason for hiding this comment

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

Curiosity: why do we import this here instead of the file's top? Any specific technical reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To avoid a circular dependency

@sbidoul
Copy link
Member

sbidoul commented Jun 11, 2025

Hi! What's the use case for this? Could this be done without changing the queue_job API?

@ivantodorovich
Copy link
Contributor Author

Hi! What's the use case for this? Could this be done without changing the queue_job API?

Hey @sbidoul 👋🏻

The case we're implementing this for is a connector, where jobs handling the import of records identify missing record dependencies that must be imported before, and so the jobs to import such dependencies are scheduled and the current job is retried after they have succeeded

def import_sale_order(self):
    ...
    missing_products = ...
    if missing_products:
        raise RetryableJobError(
            "Missing products",
            add_depends=[
                self.with_delay().import_product(missing_product)
                for missing_product in missing_products
            ],
        )

I thought it'd be an useful addition to the API

Could this be done without changing the queue_job API?

The difficult part would be that the current transaction is rolledback, and the executing Job instance -not accessible from outside- will be stored using a separate cursor, from methods that are not easy to override. I believe @SilvioC2C tried to achieve it with #758, but finally we chose another path at that time

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.

4 participants