[App Configuration] - Head request support#45858
[App Configuration] - Head request support#45858mrm9084 wants to merge 16 commits intoAzure:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a HEAD-based “check” API to Azure App Configuration’s Python client to support efficient change detection via page-level ETags (mirroring the JS SDK behavior).
Changes:
- Introduces
check_configuration_settings()on sync and async clients, backed by new generated HEAD operations. - Refactors generated paging/request preparation helpers and adds
check_key_values_in_one_pageimplementations. - Adds new sync/async AAD tests plus a new usage sample; updates changelog and test recordings assets tag.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_azure_appconfiguration_client.py | Adds sync check_configuration_settings() surface. |
| sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/aio/_azure_appconfiguration_client_async.py | Adds async check_configuration_settings() surface. |
| sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_generated/_operations/_patch.py | Adds sync generated HEAD operation + shared request/error-map helpers. |
| sdk/appconfiguration/azure-appconfiguration/azure/appconfiguration/_generated/aio/_operations/_patch.py | Adds async generated HEAD operation + shared request/error-map helpers. |
| sdk/appconfiguration/azure-appconfiguration/tests/test_azure_appconfiguration_client_aad.py | Adds sync AAD test covering page-level ETag monitoring via HEAD. |
| sdk/appconfiguration/azure-appconfiguration/tests/test_azure_appconfiguration_client_aad_async.py | Adds async AAD test covering page-level ETag monitoring via HEAD. |
| sdk/appconfiguration/azure-appconfiguration/samples/check_configuration_settings_sample.py | New sample demonstrating HEAD-based change checks via ETags. |
| sdk/appconfiguration/azure-appconfiguration/CHANGELOG.md | Documents the new check_configuration_settings() feature. |
| sdk/appconfiguration/azure-appconfiguration/assets.json | Updates assets tag for new recordings. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| match_condition: Optional[MatchConditions] = None, | ||
| continuation_token: Optional[str] = None, | ||
| **kwargs: Any, | ||
| ) -> dict: |
There was a problem hiding this comment.
Can you help explain the return type here? Is that because we return something that has @nextLink ?
There was a problem hiding this comment.
It's a dictionary with two items, nextlink and items. I'll update the doc.
| accept_datetime: Optional[Union[datetime, str]] = None, | ||
| fields: Optional[List[Union[str, ConfigurationSettingFields]]] = None, | ||
| **kwargs: Any, | ||
| ) -> ConfigurationSettingPaged: |
There was a problem hiding this comment.
I'd expect we return the same thing that list does, callers just need to know that the iterable will be empty.
There was a problem hiding this comment.
return ItemPaged[ConfigurationSetting]
There was a problem hiding this comment.
@jimmyca15 I actually looked into this a bit more and we need to return ConfigurationSettingPaged or we/our customers will have typing issues. This includes updating list to also return this. Note this isn't a breaking change.
There was a problem hiding this comment.
Type checking fails on iterator = items.by_page(match_conditions=etags) as it doesn't see match_conditions as a valid input.
There was a problem hiding this comment.
So we need to make ConfigurationSettingPaged public?
There was a problem hiding this comment.
Yes, if you look at the diff since your last review you should see me making it public.
Description
Adds a way to check for app config changes via head request.
See JS version: Azure/azure-sdk-for-js#36959
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines