Skip to content

Conversation

@mrm9084
Copy link
Member

@mrm9084 mrm9084 commented Dec 8, 2025

Description

Similar to #44247, but list_configuration_settings only returns the changed pages.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

Copilot AI review requested due to automatic review settings December 8, 2025 22:19
@github-actions github-actions bot added the App Configuration Azure.ApplicationModel.Configuration label Dec 8, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enhances the list_configuration_settings() method in Azure App Configuration SDK to support efficient monitoring of configuration changes using etag-based pagination. Instead of fetching all pages, only pages that have changed since the provided etags are returned, reducing unnecessary data transfer.

Key changes:

  • Added match_conditions parameter to by_page() method for providing etags to check against
  • Implemented custom page iterators that perform HTTP conditional requests using If-None-Match headers
  • Updated tests to use the new etag-based change detection instead of manual HTTP requests

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
CHANGELOG.md Documents the new etags parameter (should be match_conditions) for the by_page() method
azure/appconfiguration/_models.py Adds new classes ConfigurationSettingPaged, ConfigurationSettingPagedAsync, and etag-checking page iterators to support conditional page fetching
azure/appconfiguration/_azure_appconfiguration_client.py Updates sync client to return ConfigurationSettingPaged instead of ItemPaged
azure/appconfiguration/aio/_azure_appconfiguration_client_async.py Updates async client to return ConfigurationSettingPagedAsync instead of AsyncItemPaged
tests/test_azure_appconfiguration_client.py Refactors test to use the new by_page(match_conditions=...) API instead of manual HTTP requests; changes from add_configuration_setting to set_configuration_setting
tests/test_azure_appconfiguration_client_async.py Refactors async test to use the new by_page(match_conditions=...) API instead of manual HTTP requests

Comment on lines +592 to +596
super(ConfigurationSettingPropertiesPaged, self).__init__(
self._get_next_cb,
self._extract_data_cb,
continuation_token=kwargs.get("continuation_token"),
)
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] While moving the super().init() call after setting instance variables is a valid pattern that avoids potential issues with accessing uninitialized attributes, this change was not mentioned in the PR description. The change appears to be a preventative fix rather than addressing a specific bug. Consider documenting why this reordering was necessary in the commit message or PR description for future reference.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

App Configuration Azure.ApplicationModel.Configuration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant