json cell: add timeout parameter (#17920)

... and handle requests errors, consequently
This commit is contained in:
Thomas NOËL 2017-08-07 16:35:30 +02:00 committed by Frédéric Péters
parent 04f1fd1d82
commit 2d394d0221
2 changed files with 45 additions and 11 deletions

View File

@ -832,6 +832,7 @@ class JsonCellBase(CellBase):
varnames = None
force_async = False
log_errors = True
timeout = 28 # combo wsgi service timeout - 2 seconds
actions = {}
additional_data = None
# [
@ -859,7 +860,7 @@ class JsonCellBase(CellBase):
self._json_content = None
data_urls = [{'key': 'json', 'url': self.url, 'cache_duration': self.cache_duration,
'log_errors': self.log_errors}]
'log_errors': self.log_errors, 'timeout': self.timeout}]
data_urls.extend(self.additional_data or [])
for data_url_dict in data_urls:
@ -867,22 +868,35 @@ class JsonCellBase(CellBase):
for data_url_dict in data_urls:
data_key = data_url_dict['key']
log_errors = data_url_dict.get('log_errors', self.log_errors)
try:
url = utils.get_templated_url(data_url_dict['url'], context)
except utils.UnknownTemplateVariableError:
logger = logging.getLogger(__name__)
logger.warning('unknown variable in template URL (%s)', self.url)
continue
json_response = utils.requests.get(url,
headers={'Accept': 'application/json'},
remote_service='auto',
cache_duration=data_url_dict.get('cache_duration', self.cache_duration),
without_user=True,
raise_if_not_cached=not(context.get('synchronous')),
invalidate_cache=invalidate_cache,
log_errors=data_url_dict.get('log_errors', self.log_errors),
)
extra_context[data_key + '_url'] = url
try:
json_response = utils.requests.get(url,
headers={'Accept': 'application/json'},
remote_service='auto',
cache_duration=data_url_dict.get('cache_duration', self.cache_duration),
without_user=True,
raise_if_not_cached=not(context.get('synchronous')),
invalidate_cache=invalidate_cache,
log_errors=log_errors,
timeout=data_url_dict.get('timeout', self.timeout),
)
except requests.RequestException as e:
extra_context[data_key + '_status'] = -1
extra_context[data_key + '_error'] = unicode(e)
extra_context[data_key + '_exception'] = e
logger = logging.getLogger(__name__)
if log_errors:
logger.warning(u'error on request %r: %s', url, unicode(e))
else:
logger.debug(u'error on request %r: %s', url, unicode(e))
continue
extra_context[data_key + '_status'] = json_response.status_code
if json_response.status_code // 100 == 2:
if json_response.status_code != 204: # 204 = No Content
@ -930,6 +944,7 @@ class JsonCellBase(CellBase):
raise PermissionDenied()
error_message = self.actions[action].get('error-message')
timeout = self.actions[action].get('timeout', self.timeout)
logger = logging.getLogger(__name__)
content = {}
@ -984,6 +999,7 @@ class JsonCell(JsonCellBase):
varnames_str = models.CharField(_('Variable names'), max_length=200, blank=True,
help_text=_('Comma separated list of query-string variables '
'to be copied in template context'))
timeout = models.PositiveIntegerField(_('Request timeout'), default=28)
class Meta:
verbose_name = _('JSON Feed')
@ -1043,6 +1059,10 @@ class ConfigJsonCell(JsonCellBase):
def log_errors(self):
return settings.JSON_CELL_TYPES[self.key].get('log_errors',
JsonCellBase.log_errors)
@property
def timeout(self):
return settings.JSON_CELL_TYPES[self.key].get('timeout',
JsonCellBase.timeout)
@property
def actions(self):

View File

@ -186,6 +186,16 @@ def test_json_cell():
assert context['json_status'] == 404
assert context['json_error'] == data
def mocked_requests_connection_error(*args, **kwargs):
raise requests.ConnectionError('boom')
requests_get.side_effect = mocked_requests_connection_error
context = cell.get_cell_extra_context({})
assert context['json'] is None
assert context['json_status'] == -1
assert context['json_url'] == 'http://test2'
assert context['json_error'] == 'boom'
assert isinstance(context['json_exception'], requests.ConnectionError)
with pytest.raises(NothingInCacheException):
cell.url = 'http://test3'
cell.render({})
@ -356,6 +366,7 @@ def test_config_json_cell_with_param_in_url(app):
'name': 'Foobar',
'url': 'http://foo?var=[identifier]',
'log_errors': False,
'timeout': 42,
'form': [
{
"varname": "identifier",
@ -382,6 +393,7 @@ def test_config_json_cell_with_param_in_url(app):
assert requests_get.call_count == 1
assert requests_get.call_args[0][0] == 'http://foo?var=plop'
assert requests_get.call_args[-1]['log_errors'] == False
assert requests_get.call_args[-1]['timeout'] == 42
def test_json_force_async():
cell = JsonCellBase()
@ -423,7 +435,7 @@ def test_config_json_cell_additional_url(app):
'name': 'Foobar',
'url': 'http://foo',
'additional-data': [
{'key': 'plop', 'url': 'http://bar', 'log_errors': False},
{'key': 'plop', 'url': 'http://bar', 'log_errors': False, 'timeout': 42},
]
}},
TEMPLATE_DIRS=['%s/templates-1' % os.path.abspath(os.path.dirname(__file__))]):
@ -444,8 +456,10 @@ def test_config_json_cell_additional_url(app):
assert len(requests_get.mock_calls) == 2
assert requests_get.mock_calls[0][1][0] == 'http://foo'
assert requests_get.mock_calls[0][-1]['log_errors'] == True
assert requests_get.mock_calls[0][-1]['timeout'] == 28
assert requests_get.mock_calls[1][1][0] == 'http://bar'
assert requests_get.mock_calls[1][-1]['log_errors'] == False
assert requests_get.mock_calls[1][-1]['timeout'] == 42
with mock.patch('combo.utils.requests.get') as requests_get:
data = {'data': 'toto'}