From 7d668cd9f51ec289b973cbe170df9e954cc2fe06 Mon Sep 17 00:00:00 2001 From: Ilia Kurenkov Date: Wed, 28 Aug 2024 12:08:53 +0200 Subject: [PATCH 1/2] Convert an integration test to unit test. This test's flakiness wasn't worth the value it was delivering, i.e. testing that the requests lib respects the timeout. We've refocused the test on checking that we provide the timeout correctly to requests and that we let the timeout error bubble up. --- .../datadog_checks/base/utils/http.py | 4 +-- .../tests/base/utils/http/test_http.py | 28 ++++++++++++++----- 2 files changed, 23 insertions(+), 9 deletions(-) diff --git a/datadog_checks_base/datadog_checks/base/utils/http.py b/datadog_checks_base/datadog_checks/base/utils/http.py index 0e1d8e62e49d5..a53b070e94b41 100644 --- a/datadog_checks_base/datadog_checks/base/utils/http.py +++ b/datadog_checks_base/datadog_checks/base/utils/http.py @@ -156,7 +156,7 @@ class RequestsWrapper(object): 'tls_protocols_allowed', ) - def __init__(self, instance, init_config, remapper=None, logger=None): + def __init__(self, instance, init_config, remapper=None, logger=None, session=None): self.logger = logger or LOGGER default_fields = dict(STANDARD_FIELDS) @@ -329,7 +329,7 @@ def __init__(self, instance, init_config, remapper=None, logger=None): # https://requests.readthedocs.io/en/latest/user/advanced/#session-objects # https://requests.readthedocs.io/en/latest/user/advanced/#keep-alive self.persist_connections = self.tls_use_host_header or is_affirmative(config['persist_connections']) - self._session = None + self._session = session # Whether or not to log request information like method and url self.log_requests = is_affirmative(config['log_requests']) diff --git a/datadog_checks_base/tests/base/utils/http/test_http.py b/datadog_checks_base/tests/base/utils/http/test_http.py index 9273bc38daac1..3e57334785640 100644 --- a/datadog_checks_base/tests/base/utils/http/test_http.py +++ b/datadog_checks_base/tests/base/utils/http/test_http.py @@ -117,6 +117,27 @@ def test_attributes(self): assert hasattr(http.session, key) assert getattr(http.session, key) == value + def test_timeout(self): + """ + Respect the request timeout. + + Here we test two things: + - We pass the timemout option correctly to the requests library. + - We let the requests.exceptions.Timeout bubble up from the requests library. + + We trust requests to respect the timeout so we mock its response. + """ + + mock_session = mock.create_autospec(requests.Session) + mock_session.get.side_effect = requests.exceptions.Timeout() + http = RequestsWrapper({'persist_connections': True}, {'timeout': 0.08}, session=mock_session) + + with pytest.raises(requests.exceptions.Timeout): + http.get('https://foobar.com') + + assert 'timeout' in mock_session.get.call_args.kwargs, mock_session.get.call_args.kwargs + assert mock_session.get.call_args.kwargs['timeout'] == (0.08, 0.08) + class TestLogger: def test_default(self, caplog): @@ -174,10 +195,3 @@ def test_instance_override(self, caplog): expected_message = 'Sending GET request to https://www.google.com' for _, _, message in caplog.record_tuples: assert message != expected_message - - -class TestIntegration: - def test_session_timeout(self): - http = RequestsWrapper({'persist_connections': True}, {'timeout': 0.08}) - with pytest.raises(requests.exceptions.Timeout): - http.get('https://httpbin.org/delay/0.10') From 9ca89827d2339428e54aebd70eb62acc60340f07 Mon Sep 17 00:00:00 2001 From: Ilia Kurenkov Date: Wed, 28 Aug 2024 12:13:55 +0200 Subject: [PATCH 2/2] add changelog --- datadog_checks_base/changelog.d/18438.added | 1 + 1 file changed, 1 insertion(+) create mode 100644 datadog_checks_base/changelog.d/18438.added diff --git a/datadog_checks_base/changelog.d/18438.added b/datadog_checks_base/changelog.d/18438.added new file mode 100644 index 0000000000000..ec58102ba8718 --- /dev/null +++ b/datadog_checks_base/changelog.d/18438.added @@ -0,0 +1 @@ +`utils.http.RequestsWrapper` accepts a session at initialization, useful for testing and controlling sessions in general.