Skip to content
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.

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

Swagger change for getting all availability sets in a subscription and StandardSSD #3302

Merged
merged 5 commits into from
Jun 30, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,34 @@
}
}
}
},
"/subscriptions/{subscriptionId}/providers/Microsoft.Compute/availabilitySets": {
"get": {
"tags": [
"AvailabilitySets"
],
"operationId": "AvailabilitySets_ListBySubscription",
"description": "Lists all availability sets in a subscription.",
"parameters": [
{
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/SubscriptionIdParameter"
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/AvailabilitySetListResult"
}
}
},
"x-ms-pageable": {
"nextLinkName": "nextLink"
Copy link
Member

Choose a reason for hiding this comment

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

The provided example creates a linter error:

The property 'nextLink' specified by nextLinkName does not exist in the 200 response schema. \nPlease, specify the name of the property that provides the nextLink. If the model does not have the nextLink property then specify null.

Could you either:

  • Update the example to include an example of nextLink if server supports it
  • Update the Swagger to replace "nextLink" by null if server does not support nextLink

Whatever you do, do NOT remove "x-ms-pageable", that's the right thing to do since it's a method that has the semantic of a list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you please check again? I think the issue is not that nextLink is missing in the example, but that nextLink is missing AvailabilitySetListResult in compute.json.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I see what you mean, you re-used AvailabilitySetListResult as it was defined. Let me check what should be the correct way to remove this warning then.

Copy link
Member

@lmazuel lmazuel Jun 28, 2018

Choose a reason for hiding this comment

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

@dochung4 I made an actual call to the endpoint, and didn't get a nextLink back in the JSON. But maybe I didn't have enough resource to trigger the nextLink. But if it's the truth and the endpoint always returns a unique page whatever the number of AS, then the correct definition is to use null.
Could you confirm the actual server behavior:

  • If server never returns nextLink, use null
  • If server may return nextLink, use what you did nextLink

Swagger must described accurately the server, and since I don't own the server implementation I don't know which one is the truth. Your last commit changed a lot of null to nextLink and that concerns me a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For which call did you not get a nextLink back? Currently we have nextLink for listing all availability sets in a subscription, but not for listing all resource groups in a resource group. But I discussed this with my team and I added nextLink to the latter too, because it will be enabled very soon.

Please let me know if you have other thoughts.

Copy link
Member

@lmazuel lmazuel Jun 28, 2018

Choose a reason for hiding this comment

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

I checked listing AS in a RG indeed. So if I understand you correctly:

  • List AS at the subscription level always returns nextLink
  • List AS at the RG level didn't have nextLink, but you're adding it live on all api-versions from 2016-04-30-preview to now soon.

Is this accurate?
If this is accurate, I'm good with the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that's correct.

}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/availabilitySets": {
"get": {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
{
"parameters": {
"subscriptionId": "{subscriptionId}",
"api-version": "2016-04-30-preview"
},
"responses": {
"200": {
"body": {
"value": [
{
"properties": {
"platformUpdateDomainCount": 5,
"platformFaultDomainCount": 2
},
"type": "Microsoft.Compute/availabilitySets",
"location": "centralus",
"tags": {
"{tagName}": "{tagValue}"
},
"id": "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/availabilitySets/{availabilitySetName}",
"name": "{availabilitySetName}",
"sku": {
"name": "Aligned"
}
},
{
"properties": {
"platformUpdateDomainCount": 3,
"platformFaultDomainCount": 2
},
"type": "Microsoft.Compute/availabilitySets",
"location": "westus",
"tags": {
"{tagName}": "{tagValue}"
},
"id": "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/availabilitySets/{availabilitySetName}",
"name": "{availabilitySetName}",
"sku": {
"name": "Classic"
}
}
]
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,34 @@
}
}
}
},
"/subscriptions/{subscriptionId}/providers/Microsoft.Compute/availabilitySets": {
"get": {
"tags": [
"AvailabilitySets"
],
"operationId": "AvailabilitySets_ListBySubscription",
"description": "Lists all availability sets in a subscription.",
"parameters": [
{
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/SubscriptionIdParameter"
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/AvailabilitySetListResult"
}
}
},
"x-ms-pageable": {
"nextLinkName": "nextLink"
}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/availabilitySets": {
"get": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,34 @@
}
}
}
},
"/subscriptions/{subscriptionId}/providers/Microsoft.Compute/availabilitySets": {
"get": {
"tags": [
"AvailabilitySets"
],
"operationId": "AvailabilitySets_ListBySubscription",
"description": "Lists all availability sets in a subscription.",
"parameters": [
{
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/SubscriptionIdParameter"
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/AvailabilitySetListResult"
}
}
},
"x-ms-pageable": {
"nextLinkName": "nextLink"
}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/availabilitySets": {
"get": {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
{
"parameters": {
"subscriptionId": "{subscriptionId}",
"api-version": "2017-03-30"
},
"responses": {
"200": {
"body": {
"value": [
{
"properties": {
"platformUpdateDomainCount": 5,
"platformFaultDomainCount": 2
},
"type": "Microsoft.Compute/availabilitySets",
"location": "centralus",
"tags": {
"{tagName}": "{tagValue}"
},
"id": "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/availabilitySets/{availabilitySetName}",
"name": "{availabilitySetName}",
"sku": {
"name": "Aligned"
}
},
{
"properties": {
"platformUpdateDomainCount": 3,
"platformFaultDomainCount": 2
},
"type": "Microsoft.Compute/availabilitySets",
"location": "westus",
"tags": {
"{tagName}": "{tagValue}"
},
"id": "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/availabilitySets/{availabilitySetName}",
"name": "{availabilitySetName}",
"sku": {
"name": "Classic"
}
}
]
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,34 @@
}
}
}
},
"/subscriptions/{subscriptionId}/providers/Microsoft.Compute/availabilitySets": {
"get": {
"tags": [
"AvailabilitySets"
],
"operationId": "AvailabilitySets_ListBySubscription",
"description": "Lists all availability sets in a subscription.",
"parameters": [
{
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/SubscriptionIdParameter"
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/AvailabilitySetListResult"
}
}
},
"x-ms-pageable": {
"nextLinkName": "nextLink"
}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/availabilitySets": {
"get": {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
{
"parameters": {
"subscriptionId": "{subscriptionId}",
"api-version": "2017-12-01"
},
"responses": {
"200": {
"body": {
"value": [
{
"properties": {
"platformUpdateDomainCount": 5,
"platformFaultDomainCount": 2
},
"type": "Microsoft.Compute/availabilitySets",
"location": "centralus",
"tags": {
"{tagName}": "{tagValue}"
},
"id": "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/availabilitySets/{availabilitySetName}",
"name": "{availabilitySetName}",
"sku": {
"name": "Aligned"
}
},
{
"properties": {
"platformUpdateDomainCount": 3,
"platformFaultDomainCount": 2
},
"type": "Microsoft.Compute/availabilitySets",
"location": "westus",
"tags": {
"{tagName}": "{tagValue}"
},
"id": "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/availabilitySets/{availabilitySetName}",
"name": "{availabilitySetName}",
"sku": {
"name": "Classic"
}
}
]
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,34 @@
}
}
}
},
"/subscriptions/{subscriptionId}/providers/Microsoft.Compute/availabilitySets": {
"get": {
"tags": [
"AvailabilitySets"
],
"operationId": "AvailabilitySets_ListBySubscription",
"description": "Lists all availability sets in a subscription.",
"parameters": [
{
"$ref": "#/parameters/ApiVersionParameter"
},
{
"$ref": "#/parameters/SubscriptionIdParameter"
}
],
"responses": {
"200": {
"description": "OK",
"schema": {
"$ref": "#/definitions/AvailabilitySetListResult"
}
}
},
"x-ms-pageable": {
"nextLinkName": "nextLink"
}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/availabilitySets": {
"get": {
Expand Down Expand Up @@ -5081,10 +5109,11 @@
},
"StorageAccountType": {
"type": "string",
"description": "Specifies the storage account type for the managed disk. Possible values are: Standard_LRS or Premium_LRS.",
"description": "Specifies the storage account type for the managed disk. Possible values are: Standard_LRS, Premium_LRS, and StandardSSD_LRS.",
"enum": [
"Standard_LRS",
"Premium_LRS"
"Premium_LRS",
"StandardSSD_LRS"
],
"x-ms-enum": {
"name": "StorageAccountTypes",
Expand All @@ -5095,7 +5124,7 @@
"properties": {
"storageAccountType": {
"$ref": "#/definitions/StorageAccountType",
"description": "Specifies the storage account type for the managed disk. Possible values are: Standard_LRS or Premium_LRS."
"description": "Specifies the storage account type for the managed disk. Possible values are: Standard_LRS, Premium_LRS, and StandardSSD_LRS."
}
},
"allOf": [
Expand Down Expand Up @@ -5994,7 +6023,7 @@
},
"storageAccountType": {
"$ref": "#/definitions/StorageAccountType",
"description": "Specifies the storage account type for the managed disk. Possible values are: Standard_LRS or Premium_LRS."
"description": "Specifies the storage account type for the managed disk. Possible values are: Standard_LRS, Premium_LRS, and StandardSSD_LRS."
}
},
"required": [
Expand Down Expand Up @@ -6042,7 +6071,7 @@
},
"storageAccountType": {
"$ref": "#/definitions/StorageAccountType",
"description": "Specifies the storage account type for the managed disk. Possible values are: Standard_LRS or Premium_LRS."
"description": "Specifies the storage account type for the managed disk. Possible values are: Standard_LRS, Premium_LRS, and StandardSSD_LRS."
}
},
"required": [
Expand Down Expand Up @@ -6235,7 +6264,7 @@
"properties": {
"storageAccountType": {
"$ref": "#/definitions/StorageAccountType",
"description": "Specifies the storage account type for the managed disk. Possible values are: Standard_LRS or Premium_LRS."
"description": "Specifies the storage account type for the managed disk. Possible values are: Standard_LRS, Premium_LRS, and StandardSSD_LRS."
}
},
"description": "Describes the parameters of a ScaleSet managed disk."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,8 @@
"type": "string",
"enum": [
"Standard_LRS",
"Premium_LRS"
"Premium_LRS",
"StandardSSD_LRS"
Copy link
Member

Choose a reason for hiding this comment

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

This creates invaliad Swagger:
FATAL: System.InvalidOperationException: Swagger document contains two or more x-ms-enum extensions with the same name 'StorageAccountTypes' and different values: Standard_LRS,Premium_LRS vs. Standard_LRS,Premium_LRS,StandardSSD_LRS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot to update 2018-04-01 compute.json. It is updated now.

],
"x-ms-enum": {
"name": "StorageAccountTypes",
Expand All @@ -798,7 +799,7 @@
"description": "The sku tier."
}
},
"description": "The disks sku name. Can be Standard_LRS or Premium_LRS."
"description": "The disks sku name. Can be Standard_LRS, Premium_LRS, or StandardSSD_LRS."
},
"SnapshotSku": {
"properties": {
Expand Down
Loading