Skip to content

[Hub Generated] Review request for Microsoft.BotService to add version preview/2021-05-01-preview #15144

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
PhoenixHe-NV merged 7 commits into from
Jul 21, 2021
Merged

[Hub Generated] Review request for Microsoft.BotService to add version preview/2021-05-01-preview #15144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c965d58
Add disable/enable local authentication ARM property
edwin4microsoft Jul 9, 2021
a5a2dd8
Update property description
edwin4microsoft Jul 12, 2021
570cc18
add slack permission scopes
youyu16 Jul 13, 2021
5dec1be
Merge pull request #1 from youyu16/youyu16-slack-permissionscopes
bwamie Jul 13, 2021
a0f43e4
Prettify JSON
edwin4microsoft Jul 20, 2021
b0369c2
Merge branch 'edwin-botservice-Microsoft.BotService-2021-05-01-previe…
edwin4microsoft Jul 20, 2021
652439e
Make File Pretty
edwin4microsoft Jul 20, 2021
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
8 changes: 8 additions & 0 deletions ...tservice/resource-manager/Microsoft.BotService/preview/2021-05-01-preview/botservice.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1609,6 +1609,10 @@
"type": "boolean",
"description": "Whether the bot is in an isolated network"
},
"disableLocalAuth": {
"type": "boolean",
Copy link
Member

@mentat9 mentat9 Jul 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

boolean

ARM guidance is to discourage booleans in favor of string enums, e.g. localAuthType { Required, NotRequired }. These are usually easier for customers to understand and use, are extensible to more than two values, and can save complexity by eliminating invalid combinations of multiple booleans. This isn't a requirement, but you should consider it. #Resolved

Copy link
Contributor Author

@bwamie bwamie Jul 15, 2021

Choose a reason for hiding this comment

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

That is the type expected of that property. Just reusing as is per the guiding doc.

Copy link
Contributor Author

@bwamie bwamie Jul 15, 2021

Choose a reason for hiding this comment

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

That is the type expected of that property. Just reusing as is per the guiding doc.

"description": "Opt-out of local authentication and ensure only MSI and AAD can be used exclusively for authentication."
},
"schemaTransformationVersion": {
"type": "string",
"description": "The channel schema transformation version for the bot"
Expand Down Expand Up @@ -2137,6 +2141,10 @@
"type": "string",
"description": "The Slack verification token. Value only returned through POST to the action Channel List API, otherwise empty."
},
"scopes": {
Copy link
Member

@mentat9 mentat9 Jul 15, 2021

Choose a reason for hiding this comment

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

scopes

How are multiple values separated? Generally collections should not be encoded in strings if their members need to be consumed indvidually.

Copy link
Contributor Author

@bwamie bwamie Jul 15, 2021

Choose a reason for hiding this comment

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

@youyu16 can you respond to this?

Copy link
Contributor

@youyu16 youyu16 Jul 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

it will separated by ",", example, "channels:history,channels:read", this is an URL parameter.

Copy link
Member

@mentat9 mentat9 Jul 16, 2021

Choose a reason for hiding this comment

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

OK, I'd recommend updating the description to explain the format, but won't block ARM signoff.

"type": "string",
"description": "The Slack permission scopes."
},
"landingPageUrl": {
"type": "string",
"description": "The Slack landing page Url"
Expand Down
3 changes: 3 additions & 0 deletions .../resource-manager/Microsoft.BotService/preview/2021-05-01-preview/examples/CreateBot.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"isCmekEnabled": true,
"cmekKeyVaultUrl": "https://myCmekKey",
"isIsolated": false,
"disableLocalAuth": true,
"schemaTransformationVersion": "1.0"
}
}
Expand Down Expand Up @@ -72,6 +73,7 @@
"isCmekEnabled": true,
"cmekKeyVaultUrl": "https://myCmekKey",
"isIsolated": false,
"disableLocalAuth": true,
"schemaTransformationVersion": "1.0"
}
}
Expand Down Expand Up @@ -109,6 +111,7 @@
"isCmekEnabled": true,
"cmekKeyVaultUrl": "https://myCmekKey",
"isIsolated": false,
"disableLocalAuth": true,
"schemaTransformationVersion": "1.0"
}
}
Expand Down
1 change: 1 addition & 0 deletions ...ice/resource-manager/Microsoft.BotService/preview/2021-05-01-preview/examples/GetBot.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"isCmekEnabled": true,
"cmekKeyVaultUrl": "https://myCmekKey",
"isIsolated": false,
"disableLocalAuth": true,
"schemaTransformationVersion": "1.0"
}
}
Expand Down
1 change: 1 addition & 0 deletions ...ger/Microsoft.BotService/preview/2021-05-01-preview/examples/ListBotsByResourceGroup.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"isCmekEnabled": true,
"cmekKeyVaultUrl": "https://myCmekKey",
"isIsolated": false,
"disableLocalAuth": true,
"schemaTransformationVersion": "1.0"
}
}
Expand Down
1 change: 1 addition & 0 deletions ...ager/Microsoft.BotService/preview/2021-05-01-preview/examples/ListBotsBySubscription.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
"isCmekEnabled": true,
"cmekKeyVaultUrl": "https://myCmekKey",
"isIsolated": false,
"disableLocalAuth": true,
"schemaTransformationVersion": "1.0"
}
}
Expand Down
3 changes: 3 additions & 0 deletions .../resource-manager/Microsoft.BotService/preview/2021-05-01-preview/examples/UpdateBot.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"isCmekEnabled": true,
"cmekKeyVaultUrl": "https://myCmekKey",
"isIsolated": false,
"disableLocalAuth": true,
"schemaTransformationVersion": "1.0"
}
}
Expand Down Expand Up @@ -72,6 +73,7 @@
"isCmekEnabled": true,
"cmekKeyVaultUrl": "https://myCmekKey",
"isIsolated": false,
"disableLocalAuth": true,
"schemaTransformationVersion": "1.0"
}
}
Expand Down Expand Up @@ -111,6 +113,7 @@
"isCmekEnabled": true,
"cmekKeyVaultUrl": "https://myCmekKey",
"isIsolated": false,
"disableLocalAuth": true,
"schemaTransformationVersion": "1.0"
}
}
Expand Down