Skip to content

[Storage] Add Get Location Usage support #3014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
anuchandy merged 1 commit into from
May 7, 2018
Merged

[Storage] Add Get Location Usage support #3014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions ...manager/Microsoft.Storage/stable/2018-02-01/examples/StorageAccountListLocationUsage.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
{
"parameters": {
"subscriptionId": "45b60d85-fd72-427a-a708-f994d26e593e",
"location": "eastus2(stage)",
"api-version": "2018-02-01"
},
"responses": {
"200": {
"body": {
"value": [
{
"unit": "Count",
"currentValue": 55,
"limit": 250,
"name": {
"value": "StorageAccounts",
"localizedValue": "Storage Accounts"
}
}
]
}
}
}
}
40 changes: 39 additions & 1 deletion specification/storage/resource-manager/Microsoft.Storage/stable/2018-02-01/storage.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,45 @@
],
"responses": {
"200": {
"description": "OK -- current usage acount and limit retrieved and returned successfully.",
"description": "OK -- current usage count and limit retrieved and returned successfully.",
"schema": {
"$ref": "#/definitions/UsageListResult"
}
}
},
"x-ms-pageable": {
"nextLinkName": null
}
}
},
"/subscriptions/{subscriptionId}/providers/Microsoft.Storage/locations/{location}/usages": {
"get": {
"tags": [
"LocationUsage"
],
"operationId": "Usage_ListByLocation",
Copy link
Member

@anuchandy anuchandy May 4, 2018

Choose a reason for hiding this comment

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

The operation group name must be plural Usages_ListByLocation

Copy link
Member Author

@blueww blueww May 7, 2018

Choose a reason for hiding this comment

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

@anuchandy

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.

Copy link
Member Author

@blueww blueww May 7, 2018

Choose a reason for hiding this comment

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

@anuchandy
The "Usage_List" is also aligned with other features in Usage list, like Compute:

azure-rest-api-specs/specification/compute/resource-manager/Microsoft.Compute/stable/2017-12-01/compute.json

Line 1006 in de8dada

"operationId": "Usage_List",

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".

Copy link
Member

@anuchandy anuchandy May 7, 2018

Choose a reason for hiding this comment

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

@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.

Copy link
Member

@anuchandy anuchandy May 7, 2018

Choose a reason for hiding this comment

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

@blueww Created an issue #3024 and assigned to you.

"description": "Gets the current usage count and the limit for the resources of the location under the subscription.",
"x-ms-examples": {
"UsageList": { "$ref": "./examples/StorageAccountListLocationUsage.json"}
},
"parameters": [
{
"$ref": "../../../../../common-types/resource-management/v1/types.json#/parameters/ApiVersionParameter"
},
{
"$ref": "../../../../../common-types/resource-management/v1/types.json#/parameters/SubscriptionIdParameter"
},
{
"name": "location",
"in": "path",
"required": true,
"type": "string",
"description": "The location of the Azure Storage resource."
}
],
"responses": {
"200": {
"description": "OK -- current usage count and limit retrieved and returned successfully.",
"schema": {
"$ref": "#/definitions/UsageListResult"
}
Expand Down