From 8bd906c19a55465a1ff552697dd6254ef56d3096 Mon Sep 17 00:00:00 2001 From: Thomas NOEL Date: Mon, 7 Aug 2017 16:35:30 +0200 Subject: [PATCH] json cell: add timeout parameter (#17920) ... and handle requests errors, consequently --- combo/data/models.py | 40 ++++++++++++++++++++++++++++++---------- tests/test_cells.py | 16 +++++++++++++++- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/combo/data/models.py b/combo/data/models.py index 69f38e1..1d6e688 100644 --- a/combo/data/models.py +++ b/combo/data/models.py @@ -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: %', url, unicode(e)) + else: + logger.debug(u'error on request %r: %', 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): diff --git a/tests/test_cells.py b/tests/test_cells.py index b20f329..a08885f 100644 --- a/tests/test_cells.py +++ b/tests/test_cells.py @@ -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({}) @@ -342,6 +352,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", @@ -368,6 +379,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() @@ -409,7 +421,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__))]): @@ -430,8 +442,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'} -- 2.11.0