utils: do not log requests errors when the resource is down (#72914)
gitea/passerelle/pipeline/head This commit looks good
Details
gitea/passerelle/pipeline/head This commit looks good
Details
This commit is contained in:
parent
d42985797b
commit
f914b8542a
|
|
@ -254,6 +254,15 @@ def log_http_request(
|
|||
# - use a proxy for HTTP and HTTPS if resource.http_proxy exists
|
||||
|
||||
|
||||
def to_registry_key(value):
|
||||
if isinstance(value, dict):
|
||||
return tuple((key, to_registry_key(value[key])) for key in sorted(value.keys()))
|
||||
elif isinstance(value, list):
|
||||
return tuple(to_registry_key(x) for x in value)
|
||||
else:
|
||||
return value
|
||||
|
||||
|
||||
class Request(RequestSession):
|
||||
ADAPTER_REGISTRY = {} # connection pooling
|
||||
log_requests_errors = True
|
||||
|
|
@ -262,7 +271,10 @@ class Request(RequestSession):
|
|||
self.logger = kwargs.pop('logger')
|
||||
self.resource = kwargs.pop('resource', None)
|
||||
resource_log_requests_errors = getattr(self.resource, 'log_requests_errors', True)
|
||||
self.log_requests_errors = kwargs.pop('log_requests_errors', resource_log_requests_errors)
|
||||
resource_is_down = self.resource.down() if hasattr(self.resource, 'down') else False
|
||||
self.log_requests_errors = kwargs.pop(
|
||||
'log_requests_errors', resource_log_requests_errors and not resource_is_down
|
||||
)
|
||||
timeout = kwargs.pop('timeout', None)
|
||||
super().__init__(*args, **kwargs)
|
||||
if self.resource:
|
||||
|
|
@ -271,11 +283,12 @@ class Request(RequestSession):
|
|||
requests_max_retries = dict(settings.REQUESTS_MAX_RETRIES)
|
||||
if getattr(self.resource, 'requests_max_retries', None):
|
||||
requests_max_retries = dict(self.resource.requests_max_retries)
|
||||
if requests_max_retries:
|
||||
if requests_max_retries and not resource_is_down:
|
||||
requests_max_retries.setdefault('read', None)
|
||||
http_adapter_init_kwargs['max_retries'] = Retry(**requests_max_retries)
|
||||
adapter = Request.ADAPTER_REGISTRY.setdefault(
|
||||
type(self.resource), HTTPAdapter(**http_adapter_init_kwargs)
|
||||
(type(self.resource), to_registry_key(http_adapter_init_kwargs)),
|
||||
HTTPAdapter(**http_adapter_init_kwargs),
|
||||
)
|
||||
self.mount('https://', adapter)
|
||||
self.mount('http://', adapter)
|
||||
|
|
|
|||
|
|
@ -7,7 +7,8 @@ import requests
|
|||
import responses
|
||||
from django.test import override_settings
|
||||
from httmock import HTTMock, response, urlmatch
|
||||
from urllib3.exceptions import ReadTimeoutError
|
||||
from responses.registries import OrderedRegistry
|
||||
from urllib3.exceptions import ConnectionError, ReadTimeoutError
|
||||
|
||||
from passerelle.utils import CaseInsensitiveDict, Request, log_http_request
|
||||
from passerelle.utils.http_authenticators import HawkAuth
|
||||
|
|
@ -665,3 +666,74 @@ def test_requests_substitution(settings):
|
|||
requests.get('https://example.com/html?bar=foo', params={'foo': 'bar'}).text
|
||||
== '<html>\n<a href="http://example.internal/path/">\n<a/></html>'
|
||||
)
|
||||
|
||||
|
||||
@responses.activate(registry=OrderedRegistry) # pylint: disable=unexpected-keyword-arg,no-value-for-parameter
|
||||
def test_requests_resource_down():
|
||||
from passerelle.base.models import BaseResource
|
||||
|
||||
resource = mock.Mock()
|
||||
resource.requests_max_retries = {}
|
||||
resource.slug = 'test'
|
||||
resource.get_connector_slug.return_value = 'cmis'
|
||||
resource.get_settings = lambda: BaseResource.get_settings(resource)
|
||||
resource.get_setting = lambda name: BaseResource.get_setting(resource, name)
|
||||
resource.down = mock.Mock(return_value=False)
|
||||
|
||||
logger = mock.Mock()
|
||||
requests = Request(resource=resource, logger=logger)
|
||||
|
||||
responses.add(
|
||||
responses.GET,
|
||||
"https://example.com/exception",
|
||||
body=ConnectionError('down'),
|
||||
)
|
||||
with pytest.raises(ConnectionError):
|
||||
requests.get('https://example.com/exception')
|
||||
assert logger.error.call_count == 1
|
||||
assert logger.info.call_count == 0
|
||||
|
||||
responses.add(
|
||||
responses.GET,
|
||||
"https://example.com/exception",
|
||||
body=ConnectionError('down'),
|
||||
)
|
||||
logger = mock.Mock()
|
||||
resource.down.return_value = True
|
||||
requests = Request(resource=resource, logger=logger)
|
||||
with pytest.raises(ConnectionError):
|
||||
requests.get('https://example.com/exception')
|
||||
assert logger.error.call_count == 0
|
||||
assert logger.info.call_count == 1
|
||||
|
||||
responses.add(
|
||||
responses.GET,
|
||||
"https://example.com/exception",
|
||||
body='Error',
|
||||
status=500,
|
||||
)
|
||||
responses.add(
|
||||
responses.GET,
|
||||
"https://example.com/exception",
|
||||
body='ok',
|
||||
)
|
||||
resource.down.return_value = False
|
||||
resource.requests_max_retries = {
|
||||
'total': 3,
|
||||
'backoff_factor': 0.001,
|
||||
'status_forcelist': [500],
|
||||
'connect': 1,
|
||||
'read': 1,
|
||||
}
|
||||
requests = Request(resource=resource, logger=logger)
|
||||
assert requests.get('https://example.com/exception').text == 'ok'
|
||||
|
||||
responses.add(
|
||||
responses.GET,
|
||||
"https://example.com/exception",
|
||||
body='Error',
|
||||
status=500,
|
||||
)
|
||||
resource.down.return_value = True
|
||||
requests = Request(resource=resource, logger=logger)
|
||||
assert requests.get('https://example.com/exception').status_code == 500
|
||||
|
|
|
|||
Loading…
Reference in New Issue