-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Added CheckNameAvailability, Identity, systemData, used ErrorResponse v2 in DeviceUpdate #13750
Changes from 2 commits
65b58b1
3848c5f
8cd3428
b89a49c
47dee4d
9c97449
481e42f
841523a
8b4d126
59d4cbe
4df305c
3d73c55
0da41d3
f48e7f9
4d4f8f0
cac1123
9e613af
662caa2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,52 @@ | |
} | ||
}, | ||
"paths": { | ||
"/subscriptions/{subscriptionId}/providers/Microsoft.DeviceUpdate/checknameavailability": { | ||
"post": { | ||
"description": "Checks ADU resource name availability.", | ||
"operationId": "CheckNameAvailability", | ||
"parameters": [ | ||
{ | ||
"$ref": "#/parameters/ApiVersionParameter" | ||
}, | ||
{ | ||
"$ref": "#/parameters/SubscriptionIdParameter" | ||
}, | ||
{ | ||
"schema": { | ||
"$ref": "../../../../../common-types/resource-management/v2/types.json#/definitions/CheckNameAvailabilityRequest" | ||
}, | ||
"in": "body", | ||
"name": "request", | ||
"required": true, | ||
"description": "Check Name Availability Request." | ||
} | ||
], | ||
"responses": { | ||
"200": { | ||
"description": "Check Name Availability Response.", | ||
"schema": { | ||
"$ref": "../../../../../common-types/resource-management/v2/types.json#/definitions/CheckNameAvailabilityResponse" | ||
} | ||
}, | ||
"default": { | ||
"description": "Error response describing the reason for operation failure.", | ||
"schema": { | ||
"$ref": "#/definitions/ErrorDefinition" | ||
} | ||
} | ||
}, | ||
"x-ms-examples": { | ||
"CheckNameAvailability_Available": { | ||
"$ref": "./examples/CheckNameAvailability_Available.json" | ||
}, | ||
"CheckNameAvailability_AlreadyExists": { | ||
"$ref": "./examples/CheckNameAvailability_AlreadyExists.json" | ||
} | ||
}, | ||
"deprecated": false | ||
} | ||
}, | ||
"/subscriptions/{subscriptionId}/providers/Microsoft.DeviceUpdate/accounts": { | ||
"get": { | ||
"description": "Returns list of Accounts.", | ||
|
@@ -221,9 +267,6 @@ | |
} | ||
], | ||
"responses": { | ||
"200": { | ||
"description": "Async operation to delete Account was created." | ||
}, | ||
"202": { | ||
"description": "Async operation to delete Account was created." | ||
}, | ||
|
@@ -276,6 +319,12 @@ | |
"$ref": "#/definitions/Account" | ||
} | ||
}, | ||
"201": { | ||
"description": "Account updated successfully.", | ||
"schema": { | ||
"$ref": "#/definitions/Account" | ||
} | ||
}, | ||
"default": { | ||
"description": "Error response describing the reason for operation failure.", | ||
"schema": { | ||
|
@@ -455,9 +504,6 @@ | |
} | ||
], | ||
"responses": { | ||
"200": { | ||
"description": "Async operation to delete Instance was created." | ||
}, | ||
"202": { | ||
"description": "Async operation to delete Instance was created." | ||
}, | ||
|
@@ -567,6 +613,10 @@ | |
} | ||
], | ||
"properties": { | ||
"systemData": { | ||
"$ref": "../../../../../common-types/resource-management/v1/types.json#/definitions/systemData", | ||
"readOnly": true | ||
}, | ||
"properties": { | ||
"description": "Device Update account properties.", | ||
"x-ms-client-flatten": true, | ||
|
@@ -595,6 +645,10 @@ | |
"readOnly": true | ||
} | ||
} | ||
}, | ||
"identity": { | ||
"$ref": "#/definitions/Identity", | ||
"description": "The type of identity used for the resource." | ||
} | ||
} | ||
}, | ||
|
@@ -624,6 +678,10 @@ | |
} | ||
], | ||
"properties": { | ||
"systemData": { | ||
"$ref": "../../../../../common-types/resource-management/v1/types.json#/definitions/systemData", | ||
"readOnly": true | ||
}, | ||
"properties": { | ||
"description": "Device Update instance properties.", | ||
"x-ms-client-flatten": true, | ||
|
@@ -682,6 +740,33 @@ | |
} | ||
} | ||
}, | ||
"Identity": { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is available in common-types, any reason to not re-use that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The common type includes identity type enums we do not support at the moment like "UserAssigned" and "SystemAssigned, UserAssigned" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also I think the common definition did no include "None" which is a bug. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both common types v1 and v2 contain single enum value: SystemAssigned I can submit a PR to extend that. What should be the values?
Or how Swagger support multiple values? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please leave as-is for now, we are discussing making the update to the common-types files to support UserAssigned and None |
||
"description": "Identity for the resource.", | ||
"properties": { | ||
"principalId": { | ||
"readOnly": true, | ||
"type": "string", | ||
"description": "The principal ID of resource identity." | ||
}, | ||
"tenantId": { | ||
"readOnly": true, | ||
"type": "string", | ||
"description": "The tenant ID of resource." | ||
}, | ||
"type": { | ||
"enum": [ | ||
"SystemAssigned", | ||
"None" | ||
], | ||
"x-ms-enum": { | ||
"name": "ResourceIdentityType", | ||
"modelAsString": false | ||
}, | ||
"type": "string", | ||
"description": "The identity type." | ||
} | ||
} | ||
}, | ||
"IotHubSettings": { | ||
"type": "object", | ||
"description": "Device Update account integration with IoT Hub settings.", | ||
|
@@ -720,6 +805,10 @@ | |
"AccountUpdate": { | ||
"description": "Request payload used to update and existing Accounts.", | ||
"properties": { | ||
"identity": { | ||
"$ref": "#/definitions/Identity", | ||
"description": "The type of identity used for the resource." | ||
}, | ||
"location": { | ||
"type": "string", | ||
"description": "The geo-location where the resource lives" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a patch should not return a 201, but a 202 to indicate async completion - https://github.com/Azure/azure-resource-manager-rpc/blob/master/v1.0/Addendum.md#updating-using-patch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually PUT returns 201 and DELETE returns 202. In our case PATCH initiates the same flow as PUT hence 201.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
201 is Created, a PATCH cannot be used to create a resource - and hence it is not a valid status code to return
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are behind RPaaS and currently they do not support 202 for async operation only 201 even for patch. We reached out to them and they said they will work either to introduce 202 or give guidance to accept 201 for there reviews in the future. For now here is the documentation https://armwiki.azurewebsites.net/rpaas/async.html If there are any questions please reach out to RPaaS Team.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I confirmed with the RPaaS team that this is an issue today, so I will sign off. However they are looking at changing it so please work with them as to what they want you to do for this PR