[Azure App Configuration - Azure App Configuration] API Review

[azure-sdk]

New API Review meeting has been requested.

Service Name: Azure App Configuration - Azure App Configuration
Review Created By: Avani Gupta
Review Date: 10/04/2023 09:00 AM PT
Onboarding Record:
PR: #25861
Hero Scenarios Link: here
Core Concepts Doc Link: Not Provided

Description: We want to release a new API version (2023-11-01) for filtering key-values by tags.

Detailed meeting information and documents provided can be accessed here
For more information that will help prepare you for this review, the requirements, and office hours, visit the documentation here

[mikekistler]

Notes from API Review meeting 10/4/23

• The existing key and label filter syntax isn't sufficient for tags
• tags needs to specify a tag name and tag value, and tags will allow multiple params
• tags is plural because it matches the name of the property
• Is there a reason we aren't being consistent of OData, e.g. using "eq" vs "="
  • https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#filter-operators
• Should these changes be in a preview api-version ... to get feedback on the design of the feature?
• What is the grammar of the right hand side of the "eq" ? wrap strings in quotes?
• Add maxItems? You should document it but maybe not with the format keyword.
• It would be nice to have better documentation -- or a link to external docs -- on the format of the filters for key, label, tags.

Please consider the above comments but this looks good for GA.

[mikekistler]

I have added the SIgnedOff label here and will add it to the PR as well when you are ready to merge. Just ping me here or on teams when the PR is ready to merge.

[avanigupta]

@mikekistler we decided to go with the non-OData format for queries since we dont expect to support operations other than 'equals' anytime soon. So we will use tagName=tagValue format.

Also, if its okay with you, we'd prefer a stable version release for this change. But if you think preview version would be better, we can change it to preview too. Please let me know your preference.

[mikekistler]

I don't object to skipping preview giving that the changes are very small. Are you ready for me to mark the PR as signedOff? That will signal the assignee that the PR should be merged.

[avanigupta]

@mikekistler thanks for confirming! I just updated the swagger to add links to filter syntax documentation. The PR is ready to merge now.

[mikekistler]

I just realized that your PR was flagged for breaking change review. There are a bunch of ChangedParameterOrder errors, which I think are caused by added the tags parameter in the middle of the parameter list. This isn breaking at the REST API level but will likely create breaking changes in the generated client libraries.

I think you could fix the breaking changes by moving tags to the end of the parameters. If you don't want to do that, we'll need to understand what that means for your SDKs -- whether they will indeed be breaking and what the implications are for customers.

[avanigupta]

@mikekistler I dont mind moving tags to the end of parameters list. I'm expecting tag filters to be a part of SettingSelector object, just like key and label filters today. So I dont think it matters where the tags parameter is defined in the swagger, because users will not pass the filters directly to any SDK method. This is our current SDK method for getting key values using SettingSelector object:
https://learn.microsoft.com/en-us/dotnet/api/azure.data.appconfiguration.configurationclient.getconfigurationsettingsasync?view=azure-dotnet

@jsquire can you please confirm if my understanding is correct?

[jsquire]

@avanigupta: That is correct with respect to SDK packages.

[avanigupta]

@mikekistler I've updated the ordering, and the breaking change check is successful now.

[azure-sdk]

New API Review meeting has been requested.

Service Name: Azure App Configuration - Azure App Configuration
Review Created By: Avani Gupta
Review Date: 10/31/2023 01:00 PM PT
Onboarding Record:
PR: #25861
Hero Scenarios Link: here
Core Concepts Doc Link: Not Provided

Description: Following up on some additional APIs that need to be modified if we support filtering by tags.

Detailed meeting information and documents provided can be accessed here
For more information that will help prepare you for this review, the requirements, and office hours, visit the documentation here

[azure-sdk]

Cancelled by: Avani Gupta