[Storage] Add Get Location Usage support#3014
Conversation
Automation for azure-sdk-for-nodeA PR has been created for you based on this PR content. Once this PR will be merged, content will be added to your service PR: |
Automation for azure-libraries-for-javaA PR has been created for you based on this PR content. Once this PR will be merged, content will be added to your service PR: |
AutoRest linter results for ARM Related Validation Errors/WarningsThese errors are reported by the ARM team's validation tools, reachout to ARM RP API Review directly for any questions or concerns. File: specification/storage/resource-manager/readme.md
|
AutoRest linter results for SDK Related Validation Errors/WarningsThese errors are reported by the SDK team's validation tools, reachout to ADX Swagger Reviewers directly for any questions or concerns. File: specification/storage/resource-manager/readme.md
|
| Code | Id | Source | Message |
|---|---|---|---|
| OperationIdNounConflictingModelNames | R2063 | Link | OperationId has a noun that conflicts with one of the model names in definitions section. The model name will be disambiguated to 'UsageModel'. Consider using the plural form of 'Usage' to avoid this. Note: If you have already shipped an SDK on top of this spec, fixing this warning may introduce a breaking change. |
❌0 new Errors.(0 total)
AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback
Thanks for your co-operation.
Automation for azure-sdk-for-pythonA PR has been created for you based on this PR content. Once this PR will be merged, content will be added to your service PR: |
Automation for azure-sdk-for-goA PR has been created for you: |
| "tags": [ | ||
| "LocationUsage" | ||
| ], | ||
| "operationId": "Usage_ListByLocation", |
There was a problem hiding this comment.
The operation group name must be plural Usages_ListByLocation
There was a problem hiding this comment.
As there's already an API with name "Usage_List", to be aligned with this, we should use "Usage_ListByLocation" here. (Actually, only one usage of the specific location is returned with the API)
And this name already passed review in https://github.com/Azure/azure-rest-api-specs-pr/pull/459
Since the feature is target to 5/7 Build release, we need this to be merged in ASAP. I will set up a meeting tomorrow to discuss this PR if it still can't be merged.
There was a problem hiding this comment.
@anuchandy
The "Usage_List" is also aligned with other features in Usage list, like Compute:
If we want to change “Usage” to “Usages” in all APIs, it will be a breaking of existing API "Usage_List". If only change "Usage_ListByLocation" to "Usages_ListByLocation", it will be in different operation group with "Usage_List" which is wired, since 2 APIs have similar function and same return type (only scope different: one is for current subscription, one is for a location under current subscription).
I think we might can consider to change all "Usage" to "Usages" in a future release which allows breaking change. But for this release, to be aligned with existing API and no breaking, we might should keep using "Usage_ListByLocation".
There was a problem hiding this comment.
@blueww thanks! Agree with you, to have consistently with Storage::Usage_List we need Usage_ListByLocation. We can merge this PR.
Using non-plural name for operation group is against SDK guideline, seems compute spec is also violating this guideline, thanks for pointing this. Will bring this to compute team attention and get fixed.
As you mentioned in the future API version, let's fix this. Will create an issue and assign it you.
| "tags": [ | ||
| "LocationUsage" | ||
| ], | ||
| "operationId": "Usage_ListByLocation", |
There was a problem hiding this comment.
@blueww thanks! Agree with you, to have consistently with Storage::Usage_List we need Usage_ListByLocation. We can merge this PR.
Using non-plural name for operation group is against SDK guideline, seems compute spec is also violating this guideline, thanks for pointing this. Will bring this to compute team attention and get fixed.
As you mentioned in the future API version, let's fix this. Will create an issue and assign it you.
4 participants
This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.
The PR is to ass support to get Location Usage API.
The rest API doc can be found in: https://microsoft.sharepoint.com/:w:/t/AzureStorage/EYFU1AAlf3BBqV6C28ajBB0BpOXZDKOfZYnoQnZOU6wivA?e=uP50YO
The feature is target to release for Build 5/7. Please help to review it.
The change already passed review on private repo with : https://github.com/Azure/azure-rest-api-specs-pr/pull/459 (Anyway, this private PR also contains some other changes like new account properties, but other changes are postponed after Build.)
PR information
api-versionin the path should match theapi-versionin the spec).Quality of Swagger