Tag filter support #56
Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds support for tag-based filtering of settings by introducing TagFilter to Selector and associated validation. Key changes: (1) Added TagFilter field, validation logic, and comparableKey machinery to enable map key usage; (2) Updated client logic to pass TagsFilter to Azure SDK v2 and migrated imports to azappconfig/v2; (3) Expanded tests to cover tag filter validation and selector key normalization.
Reviewed Changes
Copilot reviewed 11 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| azureappconfiguration/options.go | Adds TagFilter field and comparableKey machinery for Selector. |
| azureappconfiguration/utils.go | Validates TagFilter in verifySelectors and adds validateTagFilters helper. |
| azureappconfiguration/settings_client.go | Passes TagFilter to Azure SDK selector and changes pageETags map key type. |
| azureappconfiguration/azureappconfiguration.go | Adapts ETag maps and deduplication to new selectorKey form. |
| azureappconfiguration/utils_test.go | Adds tests for tag filter validation scenarios. |
| azureappconfiguration/azureappconfiguration_test.go | Adds integration-style tests for loading with TagFilter and comparableKey behavior. |
| azureappconfiguration/snapshot_test.go | Migrates import to azappconfig/v2. |
| azureappconfiguration/refresh_test.go | Migrates import to azappconfig/v2. |
| azureappconfiguration/failover_test.go | Migrates import to azappconfig/v2. |
| azureappconfiguration/client_manager.go | Migrates import to azappconfig/v2. |
| azureappconfiguration/go.mod | Updates dependency to azappconfig/v2. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
azureappconfiguration/options.go
Outdated
| // TagFilter specifies which tags to retrieve from Azure App Configuration. | ||
| // Each tag filter must follow the format "tagName=tagValue". Only those key-values will be loaded whose tags match all the tags provided here. | ||
| // Up to 5 tag filters can be provided. If no tag filters are provided, key-values will not be filtered based on tags. | ||
| TagFilter []string |
There was a problem hiding this comment.
The name should be TagFilters, align with other languages. e.g., https://github.com/Azure/AppConfiguration-DotnetProvider/blob/34576ef70d303a029b7ea7ef860f12885fb8f35d/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/Models/KeyValueSelector.cs#L34
There was a problem hiding this comment.
Do you know why we use TagFilters in provider but TagsFilter in SDK ?
There was a problem hiding this comment.
I remember there's a thread of discussion, let me find it for you
| } | ||
|
|
||
| tagFilters := make([]string, 0) | ||
| if selector.TagFilter != "" && selector.TagFilter != "null" { |
There was a problem hiding this comment.
In what case, the TagFiliter would be "null"?
There was a problem hiding this comment.
when json.Unmarshal empty tagFilters, the tagFilter string will be "null"
There was a problem hiding this comment.
Can we do something to avoid this? E.g., do nothing if there's no element in tagFilters.
No description provided.