-
Notifications
You must be signed in to change notification settings - Fork 5.6k
misc fixes to oauth related api's on botservice swagger #3280
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
Conversation
Automation for azure-sdk-for-pythonThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-nodeNothing to generate for azure-sdk-for-node |
Automation for azure-sdk-for-javaNothing to generate for azure-sdk-for-java |
Automation for azure-sdk-for-goThe initial PR has been merged into your service PR: |
Automation for azure-sdk-for-rubyNothing to generate for azure-sdk-for-ruby |
|
Failures in CI dont seem to be due to my changes. Can someone help with this? |
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.
If you're removing resource group name, this parameter declaration should also be removed.
edit: embarrassingly commented in the wrong spot.
| } | ||
| }, | ||
| "/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.BotService/BotServices/listServiceProviders": { | ||
| "/subscriptions/{subscriptionId}/providers/Microsoft.BotService/listAuthServiceProviders": { |
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.
Please remove the other reference to the resource group parameter in this operation:
https://github.com/Azure/azure-rest-api-specs/pull/3280/files#diff-6e7e52c7a9eb6243152b8870ce21ddf4R700
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.
This is the error that is currently causing CI to fail.
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 see...thanks for pointing it out. the error in the ci build seemed puzzling.
|
Just a friendly reminder that this PR is waiting for your attention, @swagatmishra2007. :) |
|
This is a de minimis enough change that I do not believe it warrants ARM approval. Are you ready for me to merge it, @swagatmishra2007? i.e. are these changes ready for public preview consumption? |
|
@marstr yes, these changes are ready for preview. please merge (and thanks for the review!) |
added some fixes to the api swagger found while testing the api.
It involves making some items not readonly, since they can be modified by a client.
also fixed an url.