Commit 324a3ea3 authored by Nicolas Wavrant's avatar Nicolas Wavrant Committed by Kazuhiko Shiozaki

CMFActivity: do not bypass the max_retry parameter for activities that timeout

I investigated a case where activities calling an external API were not
retried after the initial failure, when it was caused by
TimeoutReachedError:

https://lab.nexedi.com/nexedi/erp5/-/blob/f41b33cd91348a800db3b95b30efd857f583bcde/bt5/erp5_web_service/MixinTemplateItem/portal_components/mixin.erp5.RESTAPIClientConnectorMixin.py#L142

The interesting thing is that these activities are written in a way that
allow them to be retried (automatically, or manually) without causing
un-intended side effects, like creating twice a document in the external
API. So not having SQLBase retrying them automatically is a pity.

The bug is that the use of TimeoutReachedError here was intended only
for the activities that are aborted by CMFActivity itself, and not for
the userland code that can also use TimeoutReachedError.

This commit removes this special processing, as there is no need to
especially not retry the activities that trigger the CMFActivity
timeout. The max_retry decision should be taken by the calling code, and
not by CMFActivity.
parent 562b35a4
...@@ -50,7 +50,7 @@ from Products.CMFActivity.ActivityRuntimeEnvironment import ( ...@@ -50,7 +50,7 @@ from Products.CMFActivity.ActivityRuntimeEnvironment import (
from .Queue import Queue, VALIDATION_ERROR_DELAY from .Queue import Queue, VALIDATION_ERROR_DELAY
from Products.CMFActivity.Errors import ActivityFlushError from Products.CMFActivity.Errors import ActivityFlushError
from Products.ERP5Type import Timeout from Products.ERP5Type import Timeout
from Products.ERP5Type.Timeout import TimeoutReachedError, Deadline from Products.ERP5Type.Timeout import Deadline
import six import six
# Stop validating more messages when this limit is reached # Stop validating more messages when this limit is reached
...@@ -1094,8 +1094,7 @@ CREATE TABLE %s ( ...@@ -1094,8 +1094,7 @@ CREATE TABLE %s (
else: else:
max_retry = m.max_retry max_retry = m.max_retry
retry = m.line.retry retry = m.line.retry
if (max_retry is not None and retry >= max_retry) or \ if max_retry is not None and retry >= max_retry:
m.exc_type == TimeoutReachedError:
# Always notify when we stop retrying. # Always notify when we stop retrying.
notify_user_list.append((m, False)) notify_user_list.append((m, False))
final_error_uid_list.append(uid) final_error_uid_list.append(uid)
......
...@@ -2794,7 +2794,6 @@ return [x.getObject() for x in context.portal_catalog(limit=100)] ...@@ -2794,7 +2794,6 @@ return [x.getObject() for x in context.portal_catalog(limit=100)]
with self.assertRaises(RuntimeError): with self.assertRaises(RuntimeError):
self.tic() self.tic()
message, = self.getMessageList('SQLDict') message, = self.getMessageList('SQLDict')
self.assertEqual(message.retry, 0)
self.deleteMessageList( self.deleteMessageList(
'SQLDict', 'SQLDict',
[message], [message],
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment