From 80db99fb1362f76e2b96d5f92e245cbbcfa0487a Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 7 Jul 2026 15:27:10 -0700 Subject: [PATCH 1/7] Bug Fixes --- .../CHANGELOG.md | 4 ++++ .../_azureappconfigurationproviderbase.py | 9 ++++++--- .../provider/_client_manager.py | 14 ++++++++++---- .../appconfiguration/provider/_discovery.py | 4 ++-- .../provider/aio/_async_client_manager.py | 18 ++++++++++++------ .../provider/aio/_async_discovery.py | 4 ++-- .../test_configuration_async_client_manager.py | 3 +++ ...ration_async_client_manager_load_balance.py | 3 +++ .../tests/test_configuration_client_manager.py | 3 +++ ...onfiguration_client_manager_load_balance.py | 3 +++ 10 files changed, 48 insertions(+), 17 deletions(-) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md b/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md index 54481723b4d3..5fe8b8e0da41 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md +++ b/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md @@ -8,6 +8,10 @@ ### Bugs Fixed +- Fixed a resource leak where replica clients that were no longer part of the auto-failover set were not closed during client refresh. +- Fixed auto-failover replica discovery so that a DNS SRV lookup timeout is distinguished from an empty replica list. A timeout now correctly triggers the longer fallback refresh interval, while an empty result refreshes at the normal interval. +- Fixed a thread-safety issue where iterating, checking membership, or getting the length of the provider's configuration mapping was not guarded by the update lock. + ### Other Changes - Bumped minimum dependency on `azure-core` to `>=1.31.0`. diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py index e5b6240d74e6..858364e4e001 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py @@ -264,17 +264,20 @@ def __getitem__(self, key: str) -> Any: return self._dict[key] def __iter__(self) -> Iterator[str]: - return self._dict.__iter__() + with self._update_lock: + return self._dict.__iter__() def __len__(self) -> int: - return len(self._dict) + with self._update_lock: + return len(self._dict) def __contains__(self, __x: object) -> bool: # pylint:disable=docstring-missing-param,docstring-missing-return,docstring-missing-rtype """ Returns True if the configuration settings contains the specified key. """ - return self._dict.__contains__(__x) + with self._update_lock: + return self._dict.__contains__(__x) def keys(self) -> KeysView[str]: """ diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py index 651a55577222..802163f11b85 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py @@ -483,10 +483,10 @@ def refresh_clients(self): if self._next_update_time and self._next_update_time > time.time(): return - failover_endpoints = find_auto_failover_endpoints(self._original_endpoint, self._replica_discovery_enabled) - - if failover_endpoints is None: - # SRV record not found, so we should refresh after a longer interval + try: + failover_endpoints = find_auto_failover_endpoints(self._original_endpoint, self._replica_discovery_enabled) + except TimeoutError: + # SRV record resolution timed out, so we should refresh after a longer interval self._next_update_time = time.time() + FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL return @@ -530,6 +530,12 @@ def refresh_clients(self): ) ) self._next_update_time = time.time() + MINIMAL_CLIENT_REFRESH_INTERVAL + # Close any replica clients that are no longer part of the failover. + retained_endpoints = {self._original_client.endpoint} + retained_endpoints.update(client.endpoint for client in discovered_clients) + for client in self._replica_clients: + if client.endpoint not in retained_endpoints: + client.close() if not self._load_balancing_enabled: random.shuffle(discovered_clients) self._replica_clients = [self._original_client] + discovered_clients diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_discovery.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_discovery.py index 6dbfe722a964..5f960a1b4e08 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_discovery.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_discovery.py @@ -40,8 +40,8 @@ def find_auto_failover_endpoints(endpoint: str, replica_discovery_enabled: bool) replicas = _find_replicas(origin.target) - if not replicas: - return None # Timeout + if replicas is None: + raise TimeoutError("Timed out while resolving auto-failover replica endpoints.") srv_records = [origin] + replicas endpoints = [] diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py index abcb6233e9a1..fba4badad8e7 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py @@ -485,12 +485,12 @@ async def refresh_clients(self): if self._next_update_time and self._next_update_time > time.time(): return - failover_endpoints = await find_auto_failover_endpoints( - self._original_endpoint, self._replica_discovery_enabled - ) - - if failover_endpoints is None: - # SRV record not found, so we should refresh after a longer interval + try: + failover_endpoints = await find_auto_failover_endpoints( + self._original_endpoint, self._replica_discovery_enabled + ) + except TimeoutError: + # SRV record resolution timed out, so we should refresh after a longer interval self._next_update_time = time.time() + FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL return @@ -534,6 +534,12 @@ async def refresh_clients(self): ) ) self._next_update_time = time.time() + MINIMAL_CLIENT_REFRESH_INTERVAL + # Close any replica clients that are no longer part of the failover. + retained_endpoints = {self._original_client.endpoint} + retained_endpoints.update(client.endpoint for client in discovered_clients) + for client in self._replica_clients: + if client.endpoint not in retained_endpoints: + await client.close() if not self._load_balancing_enabled: random.shuffle(discovered_clients) self._replica_clients = [self._original_client] + discovered_clients diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_discovery.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_discovery.py index 1713c0d0d833..ecc26d0a7e0e 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_discovery.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_discovery.py @@ -25,8 +25,8 @@ async def find_auto_failover_endpoints(endpoint: str, replica_discovery_enabled: replicas = await _find_replicas(origin.target) - if not replicas: - return None # Timeout + if replicas is None: + raise TimeoutError("Timed out while resolving auto-failover replica endpoints.") srv_records = [origin] + replicas endpoints = [] diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py index e8c2c66dc2d9..956e986637f0 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py @@ -27,6 +27,9 @@ def __init__(self, endpoint, connection_string, credential, retry_total, retry_b self.retry_total = retry_total self.retry_backoff = retry_backoff + async def close(self): + pass + @pytest.mark.usefixtures("caplog") class TestAsyncConfigurationClientManager: diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager_load_balance.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager_load_balance.py index f252798367b1..7d5ac234453a 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager_load_balance.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager_load_balance.py @@ -24,6 +24,9 @@ def __init__(self, endpoint, connection_string, credential, retry_total, retry_b self.retry_total = retry_total self.retry_backoff = retry_backoff + async def close(self): + pass + @pytest.mark.usefixtures("caplog") class TestConfigurationAsyncClientManagerLoadBalance: diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py index 5edaab158bf4..d2123f6f4f3a 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py @@ -26,6 +26,9 @@ def __init__(self, endpoint, connection_string, credential, retry_total, retry_b self.retry_total = retry_total self.retry_backoff = retry_backoff + def close(self): + pass + @pytest.mark.usefixtures("caplog") class TestConfigurationClientManager(unittest.TestCase): diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager_load_balance.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager_load_balance.py index f73942364e19..3d0fca436379 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager_load_balance.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager_load_balance.py @@ -25,6 +25,9 @@ def __init__(self, endpoint, connection_string, credential, retry_total, retry_b self.retry_total = retry_total self.retry_backoff = retry_backoff + def close(self): + pass + @pytest.mark.usefixtures("caplog") class TestConfigurationClientManagerLoadBalance(unittest.TestCase): From 08fd904f38711ace7fca57d9e600b3fa49bf4137 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 7 Jul 2026 15:56:36 -0700 Subject: [PATCH 2/7] Other small bug fixes --- .../azure-appconfiguration-provider/CHANGELOG.md | 2 ++ .../provider/_azureappconfigurationprovider.py | 2 +- .../provider/_azureappconfigurationproviderbase.py | 12 +++++++----- .../appconfiguration/provider/_client_manager.py | 4 ++-- .../provider/_key_vault/_secret_provider_base.py | 2 +- .../provider/aio/_async_client_manager.py | 4 ++-- 6 files changed, 15 insertions(+), 11 deletions(-) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md b/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md index 5fe8b8e0da41..defe7499cb04 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md +++ b/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md @@ -11,6 +11,8 @@ - Fixed a resource leak where replica clients that were no longer part of the auto-failover set were not closed during client refresh. - Fixed auto-failover replica discovery so that a DNS SRV lookup timeout is distinguished from an empty replica list. A timeout now correctly triggers the longer fallback refresh interval, while an empty result refreshes at the normal interval. - Fixed a thread-safety issue where iterating, checking membership, or getting the length of the provider's configuration mapping was not guarded by the update lock. +- Fixed `refresh_on` handling so that a single-string watched setting is treated as a key with the default (no) label instead of being incorrectly unpacked character-by-character. +- Fixed a `KeyError` when loading with an endpoint and credential (no connection string). ### Other Changes diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py index a3f8f826679c..517d25478e47 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py @@ -79,7 +79,7 @@ def __init__(self, **kwargs: Any) -> None: self._startup_timeout: int = kwargs.pop("startup_timeout", DEFAULT_STARTUP_TIMEOUT) self._replica_client_manager = ConfigurationClientManager( - connection_string=kwargs.pop("connection_string"), + connection_string=kwargs.pop("connection_string", None), endpoint=kwargs.pop("endpoint"), credential=kwargs.pop("credential", None), user_agent=user_agent, diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py index 858364e4e001..63e29a906800 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py @@ -70,11 +70,13 @@ def is_json_content_type(content_type: str) -> bool: def _build_watched_setting(setting: Union[str, Tuple[str, str]]) -> Tuple[str, str]: - try: - key, label = setting # type:ignore - except (IndexError, ValueError): - key = str(setting) # Ensure key is a string - label = NULL_CHAR + if isinstance(setting, str): + key, label = setting, NULL_CHAR + else: + try: + key, label = setting + except (TypeError, ValueError): + key, label = str(setting), NULL_CHAR if "*" in key or "*" in label: raise ValueError("Wildcard key or label filters are not supported for refresh.") return key, label diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py index 802163f11b85..013903b42f44 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py @@ -132,7 +132,7 @@ def _check_configuration_setting( self.LOGGER.debug("Refresh all triggered by key: %s label %s.", key, label) return True, None else: - raise e + raise return False, None @distributed_trace @@ -343,7 +343,7 @@ def _validate_snapshot(self, snapshot_name: str) -> bool: if e.status_code == 404: self.LOGGER.warning("Snapshot '%s' not found when resolving snapshot.", snapshot_name) return False - raise e + raise if snapshot.composition_type != SnapshotComposition.KEY: raise ValueError(f"Composition type for '{snapshot_name}' must be 'key'.") return True diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_key_vault/_secret_provider_base.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_key_vault/_secret_provider_base.py index 7b1abb9d97dc..d3a838e57c86 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_key_vault/_secret_provider_base.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_key_vault/_secret_provider_base.py @@ -31,7 +31,7 @@ def __init__(self, **kwargs: Any) -> None: ) if kwargs.get("secret_refresh_interval", 60) < 1: - raise ValueError("Secret refresh interval must be greater than 1 second.") + raise ValueError("Secret refresh interval must be at least 1 second.") self.secret_refresh_timer: Optional[_RefreshTimer] = ( _RefreshTimer(refresh_interval=kwargs.pop("secret_refresh_interval", 60)) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py index fba4badad8e7..592e045fe804 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py @@ -134,7 +134,7 @@ async def _check_configuration_setting( self.LOGGER.debug("Refresh all triggered by key: %s label %s.", key, label) return True, None else: - raise e + raise return False, None @distributed_trace @@ -345,7 +345,7 @@ async def _validate_snapshot(self, snapshot_name: str) -> bool: if e.status_code == 404: self.LOGGER.warning("Snapshot '%s' not found when resolving snapshot.", snapshot_name) return False - raise e + raise if snapshot.composition_type != SnapshotComposition.KEY: raise ValueError(f"Composition type for '{snapshot_name}' must be 'key'.") return True From 36908f2af8ab4e344f5ed07eb4d66ec793bc1ec4 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 21 Jul 2026 13:08:36 -0700 Subject: [PATCH 3/7] Added new tests --- .../tests/aio/test_async_discovery.py | 10 +++++ ...test_configuration_async_client_manager.py | 43 +++++++++++++++++++ .../test_configuration_client_manager.py | 42 ++++++++++++++++++ .../tests/test_discovery.py | 10 +++++ 4 files changed, 105 insertions(+) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_async_discovery.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_async_discovery.py index 89f4d9bafcfc..dfd739dcc7dd 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_async_discovery.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_async_discovery.py @@ -202,3 +202,13 @@ async def test_find_auto_failover_endpoints(self, mock_find_replicas, mock_find_ assert result[1] == "https://fake2.appconfig.io" mock_find_origin.assert_called_once_with("https://fake1.appconfig.io") mock_find_replicas.assert_called_once_with("fake.appconfig.io") + + # A timeout while resolving replicas is surfaced as a TimeoutError + mock_find_origin.reset_mock() + mock_find_replicas.reset_mock() + mock_find_origin.return_value = FakeAnswer(1, 99, 5000, "fake.appconfig.io") + mock_find_replicas.return_value = None + with pytest.raises(TimeoutError): + await find_auto_failover_endpoints(endpoint, True) + mock_find_origin.assert_called_once_with(endpoint) + mock_find_replicas.assert_called_once_with("fake.appconfig.io") diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py index 956e986637f0..4a9a31cd2726 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py @@ -3,12 +3,17 @@ # Licensed under the MIT License. See License.txt in the project root for # license information. # -------------------------------------------------------------------------- +import time from unittest.mock import patch, call, Mock, MagicMock import pytest from azure.appconfiguration.provider.aio._async_client_manager import ( AsyncConfigurationClientManager, _AsyncConfigurationClientWrapper, ) +from azure.appconfiguration.provider._client_manager_base import ( + FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL, + MINIMAL_CLIENT_REFRESH_INTERVAL, +) from azure.appconfiguration.provider._models import SettingSelector @@ -328,6 +333,44 @@ async def test_refresh_clients_connection_string( assert len(manager._replica_clients) == 2 mock_client.assert_not_called() + @pytest.mark.asyncio + @patch("azure.appconfiguration.provider.aio._async_client_manager.find_auto_failover_endpoints") + @patch("azure.appconfiguration.provider.aio._async_client_manager._AsyncConfigurationClientWrapper.from_credential") + async def test_refresh_clients_timeout_uses_fallback_interval(self, mock_client, mock_update_failover_endpoints): + endpoint = "https://fake.endpoint" + + mock_client.return_value = MockClient("https://fake.endpoint", "", "fake-credential", 0, 0) + mock_update_failover_endpoints.return_value = [] + manager = AsyncConfigurationClientManager(None, endpoint, "fake-credential", "", 0, 0, True, 0, 0, False) + + mock_update_failover_endpoints.reset_mock() + mock_client.reset_mock() + + # A timeout while resolving replicas falls back to the longer refresh interval + mock_update_failover_endpoints.side_effect = TimeoutError + manager._next_update_time = 0 + before = time.time() + await manager.refresh_clients() + mock_update_failover_endpoints.assert_called_once_with(endpoint, True) + # No new clients should have been created on a timeout + mock_client.assert_not_called() + assert manager._next_update_time >= before + FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL + + mock_update_failover_endpoints.reset_mock() + mock_update_failover_endpoints.side_effect = None + + # An empty result (no timeout) refreshes at the normal minimal interval + mock_update_failover_endpoints.return_value = [] + manager._next_update_time = 0 + before = time.time() + await manager.refresh_clients() + mock_update_failover_endpoints.assert_called_once_with(endpoint, True) + assert ( + before + MINIMAL_CLIENT_REFRESH_INTERVAL + <= manager._next_update_time + < before + FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL + ) + @patch("azure.appconfiguration.provider.aio._async_client_manager.find_auto_failover_endpoints") def test_calculate_backoff(self, mock_update_failover_endpoints): endpoint = "https://fake.endpoint" diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py index d2123f6f4f3a..718e804bd55d 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py @@ -3,10 +3,15 @@ # Licensed under the MIT License. See License.txt in the project root for # license information. # -------------------------------------------------------------------------- +import time import unittest from unittest.mock import patch, call, Mock, MagicMock import pytest from azure.appconfiguration.provider._client_manager import ConfigurationClientManager, _ConfigurationClientWrapper +from azure.appconfiguration.provider._client_manager_base import ( + FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL, + MINIMAL_CLIENT_REFRESH_INTERVAL, +) from azure.appconfiguration.provider._models import SettingSelector @@ -305,6 +310,43 @@ def test_refresh_clients_connection_string( assert len(manager._replica_clients) == 2 mock_client.assert_not_called() + @patch("azure.appconfiguration.provider._client_manager.find_auto_failover_endpoints") + @patch("azure.appconfiguration.provider._client_manager._ConfigurationClientWrapper.from_credential") + def test_refresh_clients_timeout_uses_fallback_interval(self, mock_client, mock_update_failover_endpoints): + endpoint = "https://fake.endpoint" + + mock_client.return_value = MockClient("https://fake.endpoint", "", "fake-credential", 0, 0) + mock_update_failover_endpoints.return_value = [] + manager = ConfigurationClientManager(None, endpoint, "fake-credential", "", 0, 0, True, 0, 0, False) + + mock_update_failover_endpoints.reset_mock() + mock_client.reset_mock() + + # A timeout while resolving replicas falls back to the longer refresh interval + mock_update_failover_endpoints.side_effect = TimeoutError + manager._next_update_time = 0 + before = time.time() + manager.refresh_clients() + mock_update_failover_endpoints.assert_called_once_with(endpoint, True) + # No new clients should have been created on a timeout + mock_client.assert_not_called() + assert manager._next_update_time >= before + FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL + + mock_update_failover_endpoints.reset_mock() + mock_update_failover_endpoints.side_effect = None + + # An empty result (no timeout) refreshes at the normal minimal interval + mock_update_failover_endpoints.return_value = [] + manager._next_update_time = 0 + before = time.time() + manager.refresh_clients() + mock_update_failover_endpoints.assert_called_once_with(endpoint, True) + assert ( + before + MINIMAL_CLIENT_REFRESH_INTERVAL + <= manager._next_update_time + < before + FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL + ) + @patch("azure.appconfiguration.provider._client_manager.find_auto_failover_endpoints") def test_calculate_backoff(self, mock_update_failover_endpoints): endpoint = "https://fake.endpoint" diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_discovery.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_discovery.py index 7e3e60b42bd2..77e2e4e352df 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_discovery.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_discovery.py @@ -198,3 +198,13 @@ def test_find_auto_failover_endpoints(self, mock_find_replicas, mock_find_origin assert result[1] == "https://fake2.appconfig.io" mock_find_origin.assert_called_once_with("https://fake1.appconfig.io") mock_find_replicas.assert_called_once_with("fake.appconfig.io") + + # A timeout while resolving replicas is surfaced as a TimeoutError + mock_find_origin.reset_mock() + mock_find_replicas.reset_mock() + mock_find_origin.return_value = FakeAnswer(1, 99, 5000, "fake.appconfig.io") + mock_find_replicas.return_value = None + with pytest.raises(TimeoutError): + find_auto_failover_endpoints(endpoint, True) + mock_find_origin.assert_called_once_with(endpoint) + mock_find_replicas.assert_called_once_with("fake.appconfig.io") From 5c1ca46373dc42d726af5cdab70bd6211a84477f Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 21 Jul 2026 13:28:30 -0700 Subject: [PATCH 4/7] Fixing mutation --- .../provider/_azureappconfigurationprovider.py | 7 ++++++- .../provider/aio/_azureappconfigurationproviderasync.py | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py index 517d25478e47..6a85119642ef 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py @@ -196,7 +196,12 @@ def refresh(self, **kwargs) -> None: self._secret_provider.secret_refresh_timer and self._secret_provider.secret_refresh_timer.needs_refresh() ): - self._dict.update(self._secret_provider.refresh_secrets()) + # Publish a new dict rather than mutating in place. Readers may hold a live view or + # iterator over self._dict, and an in-place update could raise "dictionary changed + # size during iteration". Every write to self._dict must rebind to a new object. + updated_settings = dict(self._dict) + updated_settings.update(self._secret_provider.refresh_secrets()) + self._dict = updated_settings self._replica_client_manager.refresh_clients() self._replica_client_manager.find_active_clients() replica_count = self._replica_client_manager.get_client_count() - 1 diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py index 458bd10bcce8..79385c3189b3 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py @@ -208,7 +208,12 @@ async def refresh(self, **kwargs) -> None: self._secret_provider.secret_refresh_timer and self._secret_provider.secret_refresh_timer.needs_refresh() ): - self._dict.update(await self._secret_provider.refresh_secrets()) + # Publish a new dict rather than mutating in place. Readers may hold a live view or + # iterator over self._dict, and an in-place update could raise "dictionary changed + # size during iteration". Every write to self._dict must rebind to a new object. + updated_settings = dict(self._dict) + updated_settings.update(await self._secret_provider.refresh_secrets()) + self._dict = updated_settings await self._replica_client_manager.refresh_clients() self._replica_client_manager.find_active_clients() replica_count = self._replica_client_manager.get_client_count() - 1 From 5438c662b57ab91daef3b758fd6128cd6782f030 Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 21 Jul 2026 13:52:45 -0700 Subject: [PATCH 5/7] Update _azureappconfigurationproviderbase.py --- .../provider/_azureappconfigurationproviderbase.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py index 63e29a906800..b49b0828131e 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py @@ -266,20 +266,17 @@ def __getitem__(self, key: str) -> Any: return self._dict[key] def __iter__(self) -> Iterator[str]: - with self._update_lock: - return self._dict.__iter__() + return self._dict.__iter__() def __len__(self) -> int: - with self._update_lock: - return len(self._dict) + return len(self._dict) def __contains__(self, __x: object) -> bool: # pylint:disable=docstring-missing-param,docstring-missing-return,docstring-missing-rtype """ Returns True if the configuration settings contains the specified key. """ - with self._update_lock: - return self._dict.__contains__(__x) + return self._dict.__contains__(__x) def keys(self) -> KeysView[str]: """ From ba79b1d726aaa4cffe44cb90b05b9420184d439f Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Tue, 21 Jul 2026 15:20:10 -0700 Subject: [PATCH 6/7] review items --- .../provider/_client_manager.py | 5 --- .../provider/aio/_async_client_manager.py | 5 --- ...test_configuration_async_client_manager.py | 33 ++++++++++++++++++- .../test_configuration_client_manager.py | 30 +++++++++++++++++ 4 files changed, 62 insertions(+), 11 deletions(-) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py index 013903b42f44..291e3cacdff1 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py @@ -490,11 +490,6 @@ def refresh_clients(self): self._next_update_time = time.time() + FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL return - if len(failover_endpoints) == 0: - # No failover endpoints in SRV record. - self._next_update_time = time.time() + MINIMAL_CLIENT_REFRESH_INTERVAL - return - discovered_clients = [] for failover_endpoint in failover_endpoints: found_client = False diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py index 592e045fe804..cdd6093fcef0 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py @@ -494,11 +494,6 @@ async def refresh_clients(self): self._next_update_time = time.time() + FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL return - if len(failover_endpoints) == 0: - # No failover endpoints in SRV record. - self._next_update_time = time.time() + MINIMAL_CLIENT_REFRESH_INTERVAL - return - discovered_clients = [] for failover_endpoint in failover_endpoints: found_client = False diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py index 4a9a31cd2726..debcd3da7930 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py @@ -4,7 +4,7 @@ # license information. # -------------------------------------------------------------------------- import time -from unittest.mock import patch, call, Mock, MagicMock +from unittest.mock import patch, call, Mock, MagicMock, AsyncMock import pytest from azure.appconfiguration.provider.aio._async_client_manager import ( AsyncConfigurationClientManager, @@ -371,6 +371,37 @@ async def test_refresh_clients_timeout_uses_fallback_interval(self, mock_client, < before + FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL ) + @pytest.mark.asyncio + @patch("azure.appconfiguration.provider.aio._async_client_manager.find_auto_failover_endpoints") + @patch("azure.appconfiguration.provider.aio._async_client_manager._AsyncConfigurationClientWrapper.from_credential") + async def test_refresh_clients_empty_removes_replicas(self, mock_client, mock_update_failover_endpoints): + endpoint = "https://fake.endpoint" + + mock_client.return_value = MockClient("https://fake.endpoint", "", "fake-credential", 0, 0) + mock_update_failover_endpoints.return_value = [] + manager = AsyncConfigurationClientManager(None, endpoint, "fake-credential", "", 0, 0, True, 0, 0, False) + + mock_update_failover_endpoints.reset_mock() + mock_client.reset_mock() + + # Discover a replica + replica = MockClient("https://fake.endpoint2", "", "fake-credential", 0, 0) + mock_client.return_value = replica + mock_update_failover_endpoints.return_value = ["https://fake.endpoint2"] + manager._next_update_time = 0 + await manager.refresh_clients() + assert len(manager._replica_clients) == 2 + + # A subsequent successful discovery with no replicas (e.g. the last replica was deleted) + # closes and removes the discovered client, leaving only the original. + replica.close = AsyncMock() + mock_update_failover_endpoints.return_value = [] + manager._next_update_time = 0 + await manager.refresh_clients() + replica.close.assert_awaited_once() + assert len(manager._replica_clients) == 1 + assert manager._replica_clients[0] is manager._original_client + @patch("azure.appconfiguration.provider.aio._async_client_manager.find_auto_failover_endpoints") def test_calculate_backoff(self, mock_update_failover_endpoints): endpoint = "https://fake.endpoint" diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py index 718e804bd55d..dd6eb0ba34df 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py @@ -347,6 +347,36 @@ def test_refresh_clients_timeout_uses_fallback_interval(self, mock_client, mock_ < before + FALLBACK_CLIENT_REFRESH_EXPIRED_INTERVAL ) + @patch("azure.appconfiguration.provider._client_manager.find_auto_failover_endpoints") + @patch("azure.appconfiguration.provider._client_manager._ConfigurationClientWrapper.from_credential") + def test_refresh_clients_empty_removes_replicas(self, mock_client, mock_update_failover_endpoints): + endpoint = "https://fake.endpoint" + + mock_client.return_value = MockClient("https://fake.endpoint", "", "fake-credential", 0, 0) + mock_update_failover_endpoints.return_value = [] + manager = ConfigurationClientManager(None, endpoint, "fake-credential", "", 0, 0, True, 0, 0, False) + + mock_update_failover_endpoints.reset_mock() + mock_client.reset_mock() + + # Discover a replica + replica = MockClient("https://fake.endpoint2", "", "fake-credential", 0, 0) + mock_client.return_value = replica + mock_update_failover_endpoints.return_value = ["https://fake.endpoint2"] + manager._next_update_time = 0 + manager.refresh_clients() + assert len(manager._replica_clients) == 2 + + # A subsequent successful discovery with no replicas (e.g. the last replica was deleted) + # closes and removes the discovered client, leaving only the original. + replica.close = Mock() + mock_update_failover_endpoints.return_value = [] + manager._next_update_time = 0 + manager.refresh_clients() + replica.close.assert_called_once() + assert len(manager._replica_clients) == 1 + assert manager._replica_clients[0] is manager._original_client + @patch("azure.appconfiguration.provider._client_manager.find_auto_failover_endpoints") def test_calculate_backoff(self, mock_update_failover_endpoints): endpoint = "https://fake.endpoint" From 3a18797d4246cfce691d6bc0268120f2f07ece2a Mon Sep 17 00:00:00 2001 From: Matt Metcalf Date: Mon, 27 Jul 2026 09:59:23 -0700 Subject: [PATCH 7/7] review comments --- .../CHANGELOG.md | 4 +-- .../appconfiguration/provider/_discovery.py | 2 ++ .../provider/aio/_async_discovery.py | 2 ++ .../tests/aio/test_async_discovery.py | 12 ++++++- ...test_configuration_async_client_manager.py | 32 ++++++++++++++++++- .../test_azureappconfigurationproviderbase.py | 5 +++ .../test_configuration_client_manager.py | 29 ++++++++++++++++- .../tests/test_discovery.py | 12 ++++++- 8 files changed, 92 insertions(+), 6 deletions(-) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md b/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md index defe7499cb04..8451398b3b8b 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md +++ b/sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md @@ -9,8 +9,8 @@ ### Bugs Fixed - Fixed a resource leak where replica clients that were no longer part of the auto-failover set were not closed during client refresh. -- Fixed auto-failover replica discovery so that a DNS SRV lookup timeout is distinguished from an empty replica list. A timeout now correctly triggers the longer fallback refresh interval, while an empty result refreshes at the normal interval. -- Fixed a thread-safety issue where iterating, checking membership, or getting the length of the provider's configuration mapping was not guarded by the update lock. +- Fixed auto-failover replica discovery so that a DNS SRV lookup timeout (for either the origin or replica records) is distinguished from an empty replica list. A timeout now correctly triggers the longer fallback refresh interval, while an empty result refreshes at the normal interval. +- Fixed a thread-safety issue by publishing refreshed secret values through a new configuration mapping instead of mutating the existing mapping while readers may be iterating. - Fixed `refresh_on` handling so that a single-string watched setting is treated as a key with the default (no) label instead of being incorrectly unpacked character-by-character. - Fixed a `KeyError` when loading with an endpoint and credential (no connection string). diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_discovery.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_discovery.py index 5f960a1b4e08..ff111645ec39 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_discovery.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_discovery.py @@ -57,6 +57,8 @@ def _find_origin(endpoint): uri = urlparse(endpoint).hostname request = f"_origin._tcp.{uri}" srv_records = _request_record(request) + if srv_records is None: + raise TimeoutError("Timed out while resolving auto-failover origin endpoint.") if not srv_records: return None return SRVRecord(srv_records[0]) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_discovery.py b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_discovery.py index ecc26d0a7e0e..ebe76d904626 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_discovery.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_discovery.py @@ -42,6 +42,8 @@ async def _find_origin(endpoint): uri = urlparse(endpoint).hostname request = f"_origin._tcp.{uri}" srv_records = await _request_record(request) + if srv_records is None: + raise TimeoutError("Timed out while resolving auto-failover origin endpoint.") if not srv_records: return None return SRVRecord(srv_records[0]) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_async_discovery.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_async_discovery.py index dfd739dcc7dd..04de174b5919 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_async_discovery.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_async_discovery.py @@ -114,7 +114,8 @@ async def test_find_replicas(self, mock_request_record): async def test_find_origin(self, mock_request_record): endpoint = "https://fake.endpoint" mock_request_record.return_value = None - assert not await _find_origin(endpoint) + with pytest.raises(TimeoutError): + await _find_origin(endpoint) mock_request_record.assert_called_once_with("_origin._tcp.fake.endpoint") mock_request_record.reset_mock() @@ -212,3 +213,12 @@ async def test_find_auto_failover_endpoints(self, mock_find_replicas, mock_find_ await find_auto_failover_endpoints(endpoint, True) mock_find_origin.assert_called_once_with(endpoint) mock_find_replicas.assert_called_once_with("fake.appconfig.io") + + # A timeout while resolving the origin is surfaced as a TimeoutError + mock_find_origin.reset_mock() + mock_find_replicas.reset_mock() + mock_find_origin.side_effect = TimeoutError("timeout") + with pytest.raises(TimeoutError): + await find_auto_failover_endpoints(endpoint, True) + mock_find_origin.assert_called_once_with(endpoint) + mock_find_replicas.assert_not_called() diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py index debcd3da7930..904ad424f6f6 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py @@ -31,9 +31,10 @@ def __init__(self, endpoint, connection_string, credential, retry_total, retry_b self.credential = credential self.retry_total = retry_total self.retry_backoff = retry_backoff + self.closed = False async def close(self): - pass + self.closed = True @pytest.mark.usefixtures("caplog") @@ -226,6 +227,35 @@ async def test_refresh_clients_credential( assert len(manager._replica_clients) == 2 mock_client.assert_not_called() + @pytest.mark.asyncio + @patch("azure.appconfiguration.provider.aio._async_client_manager.find_auto_failover_endpoints") + @patch("azure.appconfiguration.provider.aio._async_client_manager._AsyncConfigurationClientWrapper.from_credential") + async def test_refresh_clients_closes_removed_replicas(self, mock_client, mock_update_failover_endpoints): + endpoint = "https://fake.endpoint" + mock_update_failover_endpoints.return_value = [] + mock_client.return_value = MockClient(endpoint, "", "fake-credential", 0, 0) + manager = AsyncConfigurationClientManager( + None, endpoint, _create_mock_credential(), "", 0, 0, True, 0, 0, False + ) + + original_client = MockClient(endpoint, "", "fake-credential", 0, 0) + retained_replica = MockClient("https://fake.endpoint2", "", "fake-credential", 0, 0) + removed_replica = MockClient("https://fake.endpoint3", "", "fake-credential", 0, 0) + manager._original_client = original_client + manager._replica_clients = [original_client, retained_replica, removed_replica] + + # endpoint3 is no longer part of the failover set, so its client must be closed and dropped. + mock_update_failover_endpoints.return_value = ["https://fake.endpoint2"] + manager._next_update_time = 0 + await manager.refresh_clients() + + assert removed_replica.closed is True + assert retained_replica.closed is False + assert original_client.closed is False + assert removed_replica not in manager._replica_clients + assert retained_replica in manager._replica_clients + assert original_client in manager._replica_clients + @pytest.mark.asyncio @patch("azure.appconfiguration.provider.aio._async_client_manager.find_auto_failover_endpoints") @patch( diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_azureappconfigurationproviderbase.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_azureappconfigurationproviderbase.py index 5073f2afad31..8a13a686a322 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_azureappconfigurationproviderbase.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_azureappconfigurationproviderbase.py @@ -67,6 +67,11 @@ def test_string_input(self): result = _build_watched_setting("test_key") self.assertEqual(result, ("test_key", NULL_CHAR)) + def test_two_character_string_input(self): + """Test with a two-character string input is treated as a key, not unpacked character-by-character.""" + result = _build_watched_setting("ab") + self.assertEqual(result, ("ab", NULL_CHAR)) + def test_tuple_input(self): """Test with tuple input.""" result = _build_watched_setting(("test_key", "test_label")) diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py index dd6eb0ba34df..423322f41e3b 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py @@ -30,9 +30,10 @@ def __init__(self, endpoint, connection_string, credential, retry_total, retry_b self.credential = credential self.retry_total = retry_total self.retry_backoff = retry_backoff + self.closed = False def close(self): - pass + self.closed = True @pytest.mark.usefixtures("caplog") @@ -207,6 +208,32 @@ def test_refresh_clients_credential( assert len(manager._replica_clients) == 2 mock_client.assert_not_called() + @patch("azure.appconfiguration.provider._client_manager.find_auto_failover_endpoints") + @patch("azure.appconfiguration.provider._client_manager._ConfigurationClientWrapper.from_credential") + def test_refresh_clients_closes_removed_replicas(self, mock_client, mock_update_failover_endpoints): + endpoint = "https://fake.endpoint" + mock_update_failover_endpoints.return_value = [] + mock_client.return_value = MockClient(endpoint, "", "fake-credential", 0, 0) + manager = ConfigurationClientManager(None, endpoint, _create_mock_credential(), "", 0, 0, True, 0, 0, False) + + original_client = MockClient(endpoint, "", "fake-credential", 0, 0) + retained_replica = MockClient("https://fake.endpoint2", "", "fake-credential", 0, 0) + removed_replica = MockClient("https://fake.endpoint3", "", "fake-credential", 0, 0) + manager._original_client = original_client + manager._replica_clients = [original_client, retained_replica, removed_replica] + + # endpoint3 is no longer part of the failover set, so its client must be closed and dropped. + mock_update_failover_endpoints.return_value = ["https://fake.endpoint2"] + manager._next_update_time = 0 + manager.refresh_clients() + + assert removed_replica.closed is True + assert retained_replica.closed is False + assert original_client.closed is False + assert removed_replica not in manager._replica_clients + assert retained_replica in manager._replica_clients + assert original_client in manager._replica_clients + @patch("azure.appconfiguration.provider._client_manager.find_auto_failover_endpoints") @patch("azure.appconfiguration.provider._client_manager._ConfigurationClientWrapper.from_connection_string") def test_refresh_clients_connection_string( diff --git a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_discovery.py b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_discovery.py index 77e2e4e352df..51a22a45a38c 100644 --- a/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_discovery.py +++ b/sdk/appconfiguration/azure-appconfiguration-provider/tests/test_discovery.py @@ -111,7 +111,8 @@ def test_find_replicas(self, mock_request_record): def test_find_origin(self, mock_request_record): endpoint = "https://fake.endpoint" mock_request_record.return_value = None - assert not _find_origin(endpoint) + with pytest.raises(TimeoutError): + _find_origin(endpoint) mock_request_record.assert_called_once_with("_origin._tcp.fake.endpoint") mock_request_record.reset_mock() @@ -208,3 +209,12 @@ def test_find_auto_failover_endpoints(self, mock_find_replicas, mock_find_origin find_auto_failover_endpoints(endpoint, True) mock_find_origin.assert_called_once_with(endpoint) mock_find_replicas.assert_called_once_with("fake.appconfig.io") + + # A timeout while resolving the origin is surfaced as a TimeoutError + mock_find_origin.reset_mock() + mock_find_replicas.reset_mock() + mock_find_origin.side_effect = TimeoutError("timeout") + with pytest.raises(TimeoutError): + find_auto_failover_endpoints(endpoint, True) + mock_find_origin.assert_called_once_with(endpoint) + mock_find_replicas.assert_not_called()