[19.0][IMP] queue_job: Implement on fail hook - #955
Conversation
Allows to execute a model function when the job fails and will not be retried.
|
Just for the record, there were 2 other attempts to manage this
Your approach seem cleaner tho. |
| vals = cls._get_failure_values(job, traceback_txt, orig_exception) | ||
| job.set_failed(**vals) | ||
| job.store() | ||
| job.on_fail_hook(vals) |
There was a problem hiding this comment.
shall we pass the exception obj too?
There was a problem hiding this comment.
Is there any added value to pass the raw exception instead of what's been extracted from it in _get_failure_values ❓
There was a problem hiding this comment.
yes, you can inspect the exception for additional info. Not a must have tho.
| if commit_within_job: | ||
| self.env.cr.commit() # pylint: disable=invalid-commit | ||
|
|
||
| def _test_on_fail_hook(self, **kw): |
There was a problem hiding this comment.
If only for testing,this is not needed
There was a problem hiding this comment.
I get AttributeError: <class 'odoo.addons.queue_job.models.queue_job.QueueJob'> does not have the attribute '_test_on_fail_hook' if I remove it
florentx
left a comment
florentx
left a comment
There was a problem hiding this comment.
CI fails on test_queue_job add-on.
|
IMO the test failure is correct because it runs in a temporary env that is not rolled back. |
|
|
||
| model_name = fields.Char(string="Model", readonly=True) | ||
| method_name = fields.Char(readonly=True) | ||
| on_fail_method_name = fields.Char(readonly=True) |
There was a problem hiding this comment.
I would appreciate consistency in naming: on_fail_hook + on_fail_method + on_fail_method_name is going to make some greps miss.
There was a problem hiding this comment.
Hello @amh-mw
What would you suggest?
As I take naming quite seriously, here was my rationale:
- on_fail_method is a reference to the method (eg when call
with_delay) - on_fail_method_name is the name of the method (which must be on the same model than the queue job)
- on_fail_hook is the internal function of the job that will be calling the on_fail_method
There was a problem hiding this comment.
I might rename all of them (plus on_fail_func that I just noticed) to on_fail. They're on different classes or are method arguments, so I am 85% confident they won't collide.
In honor of this pull request, I'm going to rename _on_done in #937 to check_done to reserve the on_ prefix for hooks.
There was a problem hiding this comment.
on_fail instead of on_fail_hook make sense. For the rest, is true that func is the term that is already present. But not a blocker from my POV.
5 participants
Allows to execute a model function when the job fails and will not be retried.