-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
[Hub Generated] Review request for Microsoft.IoTCentral to add version stable/2021-06-01 #16139
Changes from 8 commits
e623d5f
33dfa67
7cfb0b6
fce54f5
fe64385
14b5078
efb9585
f787af9
4786db6
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 | ||||
---|---|---|---|---|---|---|
|
@@ -533,6 +533,35 @@ | |||||
}, | ||||||
"readOnly": true | ||||||
}, | ||||||
"ManagedIdentity": { | ||||||
"description": "The managed identity for IoT Central application.", | ||||||
"title": "Managed Identity", | ||||||
"type": "object", | ||||||
"properties": { | ||||||
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. nit: Lets be a bit more descriptive on these properties. These will help users who are unfamiliar with these terms better identify how to use them E.g. 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. sure, updated! 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. FYI, ManagementIdentify can also be refer to common definition: azure-rest-api-specs/specification/common-types/resource-management/v3/managedidentity.json Line 49 in 0a2fe04
You don't have to define your own one 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. Thanks, I've made a note. Will add this in next iteration when we support user-managed identities - we don't support that today and its present in ManagedServiceIdentityType. 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. azure-rest-api-specs/specification/common-types/resource-management/v3/managedidentity.json Line 76 in 0a2fe04
This should be the one you need. 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. Thanks, updated. |
||||||
"principalId": { | ||||||
"readOnly": true, | ||||||
"type": "string", | ||||||
"description": "The principal ID of resource identity. Also known as the objectId." | ||||||
}, | ||||||
"tenantId": { | ||||||
"readOnly": true, | ||||||
"type": "string", | ||||||
"description": "The tenant ID of resource." | ||||||
}, | ||||||
"type": { | ||||||
"description": "The type of managed identity used for the resource. 'SystemAssigned' refers to an implicitly created identity, while the type 'None' will remove any identities from the service.", | ||||||
"type": "string", | ||||||
"enum": [ | ||||||
"SystemAssigned", | ||||||
"None" | ||||||
], | ||||||
"x-ms-enum": { | ||||||
"name": "ResourceIdentityType", | ||||||
"modelAsString": true | ||||||
} | ||||||
} | ||||||
} | ||||||
}, | ||||||
"AppSkuInfo": { | ||||||
"description": "Information about the SKU of the IoT Central application.", | ||||||
"type": "object", | ||||||
|
@@ -567,6 +596,10 @@ | |||||
"sku": { | ||||||
"description": "A valid instance SKU.", | ||||||
"$ref": "#/definitions/AppSkuInfo" | ||||||
}, | ||||||
"identity": { | ||||||
"description": "The managed identities for the IoT Central application.", | ||||||
"$ref": "#/definitions/ManagedIdentity" | ||||||
} | ||||||
}, | ||||||
"allOf": [ | ||||||
|
@@ -597,6 +630,10 @@ | |||||
"description": "The common properties of an IoT Central application.", | ||||||
"x-ms-client-flatten": true, | ||||||
"$ref": "#/definitions/AppProperties" | ||||||
}, | ||||||
"identity": { | ||||||
"description": "The managed identities for the IoT Central application.", | ||||||
"$ref": "#/definitions/ManagedIdentity" | ||||||
} | ||||||
} | ||||||
}, | ||||||
|
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 think there will be three different scenarios for the response
We should show that in the example.
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.
Is there precedence or other example showing all different scenarios? I noticed enum for AppState (created/suspended) does not provide all scenarios in the examples (only created in shown in examples).
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.
The response for these 3 examples are vastly different. That is why I think it may be a good idea to include it in our examples. Just something nit. Up to you to decide.