App Configuration Provider Cleanup#47801
Open
mrm9084 wants to merge 5 commits into
Open
Conversation
Mostly docstring fixes and a couple bug fixes.
github-actions
Bot
added
the
App Configuration
Contributor
There was a problem hiding this comment.
Pull request overview
This PR cleans up the Azure App Configuration Provider implementation by improving docstrings/type hints, removing unused/dead code, and refining replica discovery/refresh behavior (including closing no-longer-used replica clients and distinguishing DNS timeout vs empty replica results).
Changes:
- Improve/normalize docstrings and type hints across sync/async provider, client managers, and Key Vault secret providers.
- Refine replica discovery/refresh logic: distinguish SRV lookup timeout vs empty replica list, and close replica clients removed from the failover set.
- Clean up constants/imports and minor behavioral fixes (e.g.,
raisevsraise e, whitespace handling).
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/appconfiguration/azure-appconfiguration-provider/tests/test_utils.py | Updates assertion to match new attempts validation message. |
| sdk/appconfiguration/azure-appconfiguration-provider/tests/test_request_tracing_context.py | Updates tests to use renamed env-var constants. |
| sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager.py | Adds close() to test stub client to support new close calls. |
| sdk/appconfiguration/azure-appconfiguration-provider/tests/test_configuration_client_manager_load_balance.py | Adds close() to test stub client to support new close calls. |
| sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager.py | Adds async close() to async test stub client. |
| sdk/appconfiguration/azure-appconfiguration-provider/tests/aio/test_configuration_async_client_manager_load_balance.py | Adds async close() to async test stub client. |
| sdk/appconfiguration/azure-appconfiguration-provider/CHANGELOG.md | Documents replica-client leak fix and timeout-vs-empty discovery behavior. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_key_vault/_async_secret_provider.py | Removes unused JSON alias/import; fixes close docstring wording. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_azureappconfigurationproviderasync.py | Adds docstring for _attempt_refresh and removes unused JSON alias. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_load.py | Fixes docstring keyword typing text and removes duplicated docs. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_discovery.py | Raises TimeoutError on SRV timeout to distinguish from empty results. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/aio/_async_client_manager.py | Improves type hints/docstrings; catches timeout via exception; closes removed replicas. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_utils.py | Doc/type cleanup and backoff calc simplification; adjusts validation/error text. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_snapshot_reference_parser.py | Improves docstring and normalizes whitespace handling. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_request_tracing_context.py | Renames imported env-var constants; removes dead constants; minor loop var cleanup. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_refresh_timer.py | Simplifies backoff cap logic (removes redundant overflow check). |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_models.py | Tightens docstring typing for Key Vault options and selectors. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_load.py | Moves JSON alias local; fixes docstring keyword typing text and removes duplicated docs. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_key_vault/_secret_provider.py | Removes unused JSON alias/import; fixes close docstring wording. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_key_vault/_secret_provider_base.py | Removes unused typing aliases; clarifies validation message. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_json.py | Clarifies docstring wording for comment-stripping helper. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_discovery.py | Raises TimeoutError on SRV timeout to distinguish from empty results. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_constants.py | Renames env-var constants to uppercase; minor comment formatting. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager.py | Improves type hints/docstrings; catches timeout via exception; closes removed replicas. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_client_manager_base.py | Removes unused typing aliases; renames load-balancing param; simplifies backoff cap. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationproviderbase.py | Refines watched-setting parsing and adds locks on additional read operations. |
| sdk/appconfiguration/azure-appconfiguration-provider/azure/appconfiguration/provider/_azureappconfigurationprovider.py | Makes connection_string kw pop default-safe and adds docstring for _attempt_refresh. |
Comment on lines
113
to
+116
| # Get startup timeout | ||
| startup_timeout = kwargs.pop("startup_timeout", DEFAULT_STARTUP_TIMEOUT) | ||
| if startup_timeout < 0: | ||
| raise ValueError("Startup timeout must be greater than or equal to 0 seconds.") | ||
| raise ValueError("Startup timeout must be at least 1 second.") |
Comment on lines
268
to
+270
| def __iter__(self) -> Iterator[str]: | ||
| return self._dict.__iter__() | ||
| with self._update_lock: | ||
| return self._dict.__iter__() |
Comment on lines
41
to
45
| 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.") | ||
|
|
Comment on lines
26
to
30
| 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.") | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Mostly Updates missing/wrong doc strings and type hints.
But also:
JOSNtype in multiple locations.raiseinstead ofraise eAll SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines