BotService: Support for channel management in bots #2793

carlosscastro · 2018-04-02T03:01:52Z

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

The title of the PR is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
If applicable, the PR references the bug/issue that it fixes.
Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

I have read the contribution guidelines.
My spec meets the review criteria:
- The spec conforms to the Swagger 2.0 specification.
- The spec follows the guidelines described in the Swagger checklist document.
- Validation tools were run on swagger spec(s) and have all been fixed in this PR.

AutorestCI · 2018-04-02T03:02:18Z

Automation for azure-sdk-for-go

A PR has been created for you:
Azure/azure-sdk-for-go#1588

AutorestCI · 2018-04-02T03:02:25Z

Automation for azure-sdk-for-python

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-python#2337

AutorestCI · 2018-04-02T03:02:25Z

Automation for azure-libraries-for-java

Nothing to generate for azure-libraries-for-java

AutorestCI · 2018-04-02T03:02:29Z

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

carlosscastro · 2018-04-02T03:05:18Z

The goal of this pull request is mainly to introduce Channels, which are different means to expose a bot. Given that a bot resource has multiple channels, we have modeled the channels as a nested (and proxy only) resource of the bot.

We have multiple channels where bots can be exposed, and each has a different payload, but there is common information and machinery across channels as well.

This is why we modeled channels the following way

/botservice/{botid}/channel/{channelid}

Where channel id specifies the type of the channel, such as Facebook, Email or Skype. In the swagger , given that each channel has a specific payload, we create a base class channel and then inherited classes, making use of the "discriminator" feature of swagger.

olydis · 2018-04-02T17:28:58Z

@marstr Stole this from you since I got assigned to the original PR in private, just moved this over to public 🙂

olydis · 2018-04-02T17:32:33Z

@carlosscastro Awesome, thanks for moving this over! 🙂

olydis · 2018-04-02T18:00:11Z

@ravbhatnagar New operations (anything starting with Channels_ - I'd just look at the incoming file, the diff is messed up in the operations area...) and types DirectLineSite, WebChatSite and all types containing Channel.

Note that the diff formally indicates breaking changes (e.g. changed path of "check name availability" operation), but there are none: The RP went public as represented by the Swagger after this PR and no SDKs have been released based on the "old" Swagger. I've only migrated the old Swagger from private to public in order to accept this PR against public.

ravbhatnagar · 2018-04-04T06:48:16Z

...ification/botservice/resource-manager/Microsoft.BotService/stable/2017-12-01/botservice.json

+          "type": "string",
+          "description": "Facebook application secret"
+        },
+        "accessToken": {


Is this a secret? If yes, it should not be returned via a GET

ravbhatnagar · 2018-04-04T06:48:30Z

...ification/botservice/resource-manager/Microsoft.BotService/stable/2017-12-01/botservice.json

+          "type": "string",
+          "description": "Facebook application id"
+        },
+        "appSecret": {


Is this a secret? If yes, it should not be returned via a GET

ravbhatnagar · 2018-04-04T06:49:41Z

...ification/botservice/resource-manager/Microsoft.BotService/stable/2017-12-01/botservice.json

+          "readOnly": true,
+          "description": "Callback Url"
+        },
+        "isEnabled": {


bools are typically not recommended. Not self descriptive and dont allow for future expansion of values that can be supported. Consider - channelState or channelStatus - Enabled|Disabled and in future it can potentially support more values.

ravbhatnagar · 2018-04-04T06:50:14Z

...ification/botservice/resource-manager/Microsoft.BotService/stable/2017-12-01/botservice.json

+          "type": "string",
+          "description": "The email address"
+        },
+        "password": {


Should not be returned via a GET/PUT. It should be exposed via POST if needed.

same feedback for all other such properties in different models.

olydis · 2018-04-11T18:12:28Z

@carlosscastro please address the feedback, otherwise this PR will be closed due to inactivity (but can of course be reopened at a later point in time).

olydis · 2018-04-13T18:01:27Z

@salameer @carlosscastro Closing due to inactivity. Feel free to reopen once you are ready to address the feedback.

carlosscastro · 2018-04-16T21:01:43Z

Thanks! I was waiting for a response from Gaurav regarding the right soluiton to his comments.

swagger update

90d96a5

azuresdkci assigned marstr Apr 2, 2018

azuresdkci requested a review from marstr April 2, 2018 03:31

update wrong tag

3deb7a0

olydis assigned olydis and unassigned marstr Apr 2, 2018

olydis added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Apr 2, 2018

Merge branch 'master' into master

7383f8e

AutorestCI mentioned this pull request Apr 3, 2018

[AutoPR botservice/resource-manager] BotService: Support for channel management in bots Azure/azure-sdk-for-python#2338

Closed

carlosscastro mentioned this pull request Apr 3, 2018

New SDK: Azure Bot Service Azure/azure-sdk-for-net#4184

Merged

9 tasks

ravbhatnagar reviewed Apr 4, 2018

View reviewed changes

olydis assigned salameer and unassigned olydis Apr 13, 2018

olydis closed this Apr 13, 2018

AutorestCI mentioned this pull request Apr 13, 2018

[AutoPR botservice/resource-manager] BotService: Support for channel management in bots Azure/azure-sdk-for-go#1588

Closed

carlosscastro mentioned this pull request Apr 16, 2018

BotService: Support for channel management in bots #2887

Merged

10 tasks

BotService: Support for channel management in bots #2793

BotService: Support for channel management in bots #2793

Uh oh!

Conversation

carlosscastro commented Apr 2, 2018 • edited by azuresdkci Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR information

Quality of Swagger

Uh oh!

AutorestCI commented Apr 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-go

Uh oh!

AutorestCI commented Apr 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-python

Uh oh!

AutorestCI commented Apr 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-libraries-for-java

Uh oh!

AutorestCI commented Apr 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-node

Uh oh!

carlosscastro commented Apr 2, 2018

Uh oh!

olydis commented Apr 2, 2018

Uh oh!

olydis commented Apr 2, 2018

Uh oh!

olydis commented Apr 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ravbhatnagar Apr 4, 2018

Choose a reason for hiding this comment

Uh oh!

ravbhatnagar Apr 4, 2018

Choose a reason for hiding this comment

Uh oh!

ravbhatnagar Apr 4, 2018

Choose a reason for hiding this comment

Uh oh!

ravbhatnagar Apr 4, 2018

Choose a reason for hiding this comment

Uh oh!

ravbhatnagar Apr 4, 2018

Choose a reason for hiding this comment

Uh oh!

olydis commented Apr 11, 2018

Uh oh!

olydis commented Apr 13, 2018

Uh oh!

carlosscastro commented Apr 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

carlosscastro commented Apr 2, 2018 •

edited by azuresdkci

Loading

AutorestCI commented Apr 2, 2018 •

edited

Loading

AutorestCI commented Apr 2, 2018 •

edited

Loading

AutorestCI commented Apr 2, 2018 •

edited

Loading

AutorestCI commented Apr 2, 2018 •

edited

Loading

olydis commented Apr 2, 2018 •

edited

Loading