-
-
Notifications
You must be signed in to change notification settings - Fork 522
[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
base: 17.0
Are you sure you want to change the base?
Conversation
This commit adds the posibility to add new dependencies to a job when raising a RetryableJobError.
|
Hi @guewen, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
| self.result = self.func(*tuple(self.args), **self.kwargs) | ||
| except RetryableJobError as err: | ||
| if err.add_depends: | ||
| from .delay import DelayableGraph |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
|
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
The difficult part would be that the current transaction is rolledback, and the executing |
|
Hi @ivantodorovich , creating a new job that inserts itself in a new dependency of the current job and then retry the later which such becomes in a wait_dependencies state seems complicated and I'm afraid of all the corner cases we might encounter (job already in a graph, ...). May I suggest to handle this in the job itself ? The code of the job method may enqueue the prerequisite job, chained with a new version of itself (same job method, same parameters), and the current job terminates, so is "done". This solution also doesn't need to handle another transaction. |
|
Admittedly if the job is already in a graph it may not do what we want neither |
This commit adds the posibility to add new dependencies to a job when raising a RetryableJobError.