-
-
Notifications
You must be signed in to change notification settings - Fork 481
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
[16.0][IMP] queue_job: Add error handler when job fails #734
base: 16.0
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,6 +74,7 @@ Features: | |
description, number of retries | ||
* Related Actions: link an action on the job view, such as open the record | ||
concerned by the job | ||
* Error Handler: trigger a method when job fails, such as calling a webhook | ||
|
||
**Table of contents** | ||
|
||
|
@@ -429,6 +430,21 @@ Based on this configuration, we can tell that: | |
* retries 10 to 15 postponed 30 seconds later | ||
* all subsequent retries postponed 5 minutes later | ||
|
||
**Job function: Error Handler** | ||
|
||
The *Error Handler* is a method executed whenever the job fails | ||
|
||
It's configured similarly to Related Action | ||
|
||
There is an OOTB handler: _call_webhook, which calls a webhook with configurable information. | ||
|
||
Example of using _call_webhook to call a webhook to Slack: | ||
|
||
.. code-block:: xml | ||
|
||
<field name="error_handler" eval='{"func_name": "_call_webhook", "kwargs": {"webhook_url": "XXX", "only_if_max_retries_reached":True, "payload": {"text": "Hello World!"}}}' /> | ||
|
||
|
||
**Job Context** | ||
|
||
The context of the recordset of the job, or any recordset passed in arguments of | ||
|
@@ -687,6 +703,7 @@ Contributors | |
* Souheil Bejaoui <[email protected]> | ||
* Eric Antones <[email protected]> | ||
* Simone Orsi <[email protected]> | ||
* Tris Doan <[email protected]> | ||
|
||
Maintainers | ||
~~~~~~~~~~~ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,13 @@ | ||
# Copyright 2013-2020 Camptocamp SA | ||
# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) | ||
|
||
import json | ||
import logging | ||
import random | ||
from datetime import datetime, timedelta | ||
|
||
import requests | ||
|
||
from odoo import _, api, exceptions, fields, models | ||
from odoo.osv import expression | ||
from odoo.tools import config, html_escape | ||
|
@@ -506,3 +509,31 @@ | |
_logger.info("Running test job.") | ||
if random.random() <= failure_rate: | ||
raise JobError("Job failed") | ||
|
||
def _call_webhook(self, **kwargs): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand why we have to handle this here. If you provide your own handler you can do whatever you want, including calling a webhook. I'd drop this part. |
||
only_if_max_retries_reached = kwargs.get("only_if_max_retries_reached") | ||
job = kwargs.get("job") | ||
if only_if_max_retries_reached and job and job.retry < job.max_retries: | ||
return | ||
|
||
webhook_url = kwargs.get("webhook_url") | ||
if not webhook_url: | ||
return | ||
payload = kwargs.get("payload") | ||
json_values = json.dumps(payload, sort_keys=True, default=str) | ||
headers = kwargs.get("headers", {"Content-Type": "application/json"}) | ||
# inspired by https://github.com/odoo/odoo/blob/18.0/odoo/addons/base | ||
# /models/ir_actions.py#L867 | ||
try: | ||
response = requests.post( | ||
url=webhook_url, data=json_values, headers=headers, timeout=1 | ||
) | ||
response.raise_for_status() | ||
except requests.exceptions.ReadTimeout: | ||
_logger.warning( | ||
"Webhook call timed out after 1s - it may or may not have failed. " | ||
"If this happens often, it may be a sign that the system you're " | ||
"trying to reach is slow or non-functional." | ||
) | ||
except requests.exceptions.RequestException as exc: | ||
_logger.warning("Webhook call failed: %s", exc) | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,3 +10,4 @@ | |
* Souheil Bejaoui <[email protected]> | ||
* Eric Antones <[email protected]> | ||
* Simone Orsi <[email protected]> | ||
* Tris Doan <[email protected]> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,12 +2,15 @@ | |
# License LGPL-3.0 or later (http://www.gnu.org/licenses/lgpl.html) | ||
|
||
import hashlib | ||
import logging | ||
from datetime import datetime, timedelta | ||
from unittest import mock | ||
from unittest.mock import patch | ||
|
||
import odoo.tests.common as common | ||
|
||
from odoo.addons.queue_job import identity_exact | ||
from odoo.addons.queue_job.controllers.main import RunJobController | ||
from odoo.addons.queue_job.delay import DelayableGraph | ||
from odoo.addons.queue_job.exception import ( | ||
FailedJobError, | ||
|
@@ -24,9 +27,12 @@ | |
WAIT_DEPENDENCIES, | ||
Job, | ||
) | ||
from odoo.addons.queue_job.tests.common import trap_jobs | ||
|
||
from .common import JobCommonCase | ||
|
||
_logger = logging.getLogger(__name__) | ||
|
||
|
||
class TestJobsOnTestingMethod(JobCommonCase): | ||
"""Test Job""" | ||
|
@@ -341,6 +347,25 @@ | |
job1 = Job.load(self.env, test_job_1.uuid) | ||
self.assertEqual(job1.identity_key, expected_key) | ||
|
||
def test_failed_job_perform(self): | ||
with trap_jobs() as trap: | ||
model = self.env["test.queue.job"] | ||
job = model.with_delay(priority=1, max_retries=1).testing_method() | ||
trap.assert_jobs_count(1) | ||
with patch.object(type(job), "perform", side_effect=IOError), patch( | ||
"odoo.sql_db.Cursor.commit", return_value=None | ||
): # avoid odoo.sql_db: bad query: ROLLBACK TO SAVEPOINT test_0 | ||
controller = RunJobController() | ||
try: | ||
controller._try_perform_job(self.env, job) | ||
with patch( | ||
"odoo.addons.test_queue_job.models.test_models.QueueJob" | ||
".testing_error_handler" | ||
) as patched: | ||
patched.assert_called_once() | ||
except Exception: | ||
_logger.info("Job fails") | ||
Comment on lines
+359
to
+367
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that lines 361-365 are not executed. Test has to be improved here, I believe. Extract of the log on CI:
|
||
|
||
|
||
class TestJobs(JobCommonCase): | ||
"""Test jobs on other methods or with different job configuration""" | ||
|
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.
job should be the 1st positional argument IMO