diff --git a/changelog.d/20261106_173908_kurtmckee_fix_tune_leaks.rst b/changelog.d/20261106_173908_kurtmckee_fix_tune_leaks.rst new file mode 100644 index 000000000..dc5b1c729 --- /dev/null +++ b/changelog.d/20261106_173908_kurtmckee_fix_tune_leaks.rst @@ -0,0 +1,9 @@ +Fixed +----- + +- ``RequestsTransport.tune()`` and ``RetryConfig.tune()`` now restore the settings + even if an exception is raised. (:pr:`NUMBER`) + + Previously, an exception inside a tuned context + (like ``with client.transport.tune(verify_ssl=False):``) + would leave the modified settings configured for subsequent requests. diff --git a/src/globus_sdk/transport/requests.py b/src/globus_sdk/transport/requests.py index 97089d67c..ba96e013c 100644 --- a/src/globus_sdk/transport/requests.py +++ b/src/globus_sdk/transport/requests.py @@ -241,11 +241,13 @@ def tune( self.verify_ssl = str(verify_ssl) if http_timeout is not None: self.http_timeout = http_timeout - yield - ( - self.verify_ssl, - self.http_timeout, - ) = saved_settings + try: + yield + finally: + ( + self.verify_ssl, + self.http_timeout, + ) = saved_settings @functools.cached_property def json_provider(self) -> RequestsRepresentationProvider: diff --git a/src/globus_sdk/transport/retry_config.py b/src/globus_sdk/transport/retry_config.py index ff458865b..9422153e5 100644 --- a/src/globus_sdk/transport/retry_config.py +++ b/src/globus_sdk/transport/retry_config.py @@ -91,9 +91,11 @@ def tune( self.max_sleep = max_sleep if max_retries is not None: self.max_retries = max_retries - yield - ( - self.backoff, - self.max_sleep, - self.max_retries, - ) = saved_settings + try: + yield + finally: + ( + self.backoff, + self.max_sleep, + self.max_retries, + ) = saved_settings diff --git a/tests/unit/transport/test_transport.py b/tests/unit/transport/test_transport.py index 3a86ec83f..c0fda9141 100644 --- a/tests/unit/transport/test_transport.py +++ b/tests/unit/transport/test_transport.py @@ -71,6 +71,53 @@ def test_retry_tuning(param_name, init_value, tune_value): assert getattr(config, param_name) == init_value +class _BodyFailed(Exception): + pass + + +@pytest.mark.parametrize( + "param_name, init_value, tune_value", + [ + ("verify_ssl", True, False), + ("verify_ssl", True, str(ca_bundle_file)), + ("http_timeout", 60, 120), + ], +) +def test_transport_tuning_is_reverted_when_the_block_raises( + param_name, init_value, tune_value +): + transport = RequestsTransport(**{param_name: init_value}) + + with pytest.raises(_BodyFailed): + with transport.tune(**{param_name: tune_value}): + # guard: the tuning was in effect, so the revert is what is under test + assert getattr(transport, param_name) == tune_value + raise _BodyFailed + + assert getattr(transport, param_name) == init_value + + +@pytest.mark.parametrize( + "param_name, init_value, tune_value", + [ + ("backoff", _exponential_backoff, _linear_backoff), + ("max_sleep", 10, 1), + ("max_retries", 10, 0), + ], +) +def test_retry_tuning_is_reverted_when_the_block_raises( + param_name, init_value, tune_value +): + config = RetryConfig(**{param_name: init_value}) + + with pytest.raises(_BodyFailed): + with config.tune(**{param_name: tune_value}): + assert getattr(config, param_name) == tune_value + raise _BodyFailed + + assert getattr(config, param_name) == init_value + + def test_transport_can_manipulate_user_agent(): transport = RequestsTransport()