Skip to content
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

[Synapse] Support long running operation and add subclass of Activity in artifact #10270

Merged
merged 6 commits into from
Aug 3, 2020

Conversation

wonner
Copy link
Contributor

@wonner wonner commented Jul 28, 2020

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

If any further question about AME onboarding or validation tools, please view the FAQ.

ARM API Review Checklist

  • Ensure to check this box if one of the following scenarios meet updates in the PR, so that label “WaitForARMFeedback” will be added automatically to involve ARM API Review. Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs, all “removals” and “adding a new property” no more require ARM API review.

    • Adding new API(s)
    • Adding a new API version
    • Adding a new service
  • If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.

Breaking Change Review Checklist

If there are following updates in the PR, ensure to request an approval from API Review Board as defined in the Breaking Change Policy.

  • Removing API(s) in stable version
  • Removing properties in stable version
  • Removing API version(s) in stable version
  • Updating API in stable version with Breaking Change Validation errors
  • Updating API(s) in preview over 1 year

Please follow the link to find more details on PR review process.

@openapi-pipeline-app
Copy link

openapi-pipeline-app bot commented Jul 28, 2020

[Staging] Swagger Validation Report

BreakingChange: 9 Errors, 0 Warnings [Detail] [Expand]
Rule Message
1011 - AddingResponseCode The new version adds a response code '202'.
New: Microsoft.Synapse/preview/2019-06-01-preview/artifacts.json#L874:11
1011 - AddingResponseCode The new version adds a response code '202'.
New: Microsoft.Synapse/preview/2019-06-01-preview/artifacts.json#L613:11
1011 - AddingResponseCode The new version adds a response code '202'.
New: Microsoft.Synapse/preview/2019-06-01-preview/artifacts.json#L458:11
1011 - AddingResponseCode The new version adds a response code '202'.
New: Microsoft.Synapse/preview/2019-06-01-preview/artifacts.json#L537:11
1011 - AddingResponseCode The new version adds a response code '202'.
New: Microsoft.Synapse/preview/2019-06-01-preview/artifacts.json#L112:11
1011 - AddingResponseCode The new version adds a response code '202'.
New: Microsoft.Synapse/preview/2019-06-01-preview/artifacts.json#L191:11
1011 - AddingResponseCode The new version adds a response code '202'.
New: Microsoft.Synapse/preview/2019-06-01-preview/artifacts.json#L953:11
1011 - AddingResponseCode The new version adds a response code '202'.
New: Microsoft.Synapse/preview/2019-06-01-preview/artifacts.json#L2208:11
1011 - AddingResponseCode The new version adds a response code '202'.
New: Microsoft.Synapse/preview/2019-06-01-preview/artifacts.json#L2287:11
️✔️LintDiff [Detail]
 Validation passes for LintDiff. 
️✔️Avocado [Detail]
 Validation passes for Avocado. 
Posted by Swagger Pipeline | How to fix these errors?

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 28, 2020

azure-sdk-for-python - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 28, 2020

Azure CLI Extension Generation - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 28, 2020

azure-sdk-for-js - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 28, 2020

azure-sdk-for-go - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 28, 2020

Trenton Generation - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 28, 2020

azure-sdk-for-net - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 28, 2020

azure-sdk-for-java - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 28, 2020

azure-sdk-for-python-track2 - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@idear1203 idear1203 self-requested a review July 28, 2020 05:51
Copy link
Contributor

@idear1203 idear1203 left a comment

Choose a reason for hiding this comment

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

LGTM.

@lmazuel , could you please take a look? Thank you in advance!

In this change we do the following two things:

  1. correct some API response status code from 200 to 202. We found the status code is incorrect thus exception is thrown from SDK unexpectedly when we were writing some PowerShell cmdlets based on C# SDK. This change aims to fix the issue. So the breaking change is by design.
  2. We added a few (~9) new schema definitions. There is no API path/parameter changes.

@@ -102,8 +102,8 @@
}
],
"responses": {
"200": {
"description": "OK.",
"202": {
Copy link
Member

Choose a reason for hiding this comment

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

I need more context here, does it mean it was wrong to say that the server was returning a 200? Or did you change the API to return a 202? Could you describe a little more if your 202 is pollable (have a Location header, etc.)

We can have this discussion by email if you anticipate it to be long.
Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

We got the Swagger spec for our RP devs. According to our tests, the Swagger spec is incorrect. The server is actually returning 202 rather than 200. The response is pollable, we do have a Location header in the response. Let me know if you need more info.

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@idear1203
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@lmazuel lmazuel merged commit 4bb6de0 into Azure:master Aug 3, 2020
iscai-msft added a commit to iscai-msft/azure-rest-api-specs that referenced this pull request Oct 5, 2020
…into fix_kv_python_readme

* 'master' of https://github.com/Azure/azure-rest-api-specs: (29 commits)
  Add preAttack tactic to swagger (Azure#10332)
  Add Url Signing action definition (Azure#10326)
  [Synapse] Support long running operation and add subclass of Activity in artifact (Azure#10270)
  Add native blob soft delete (Azure#10308)
  Fix Microsoft.Resources naming issue (Azure#10318)
  Notebooks swagger spec (Azure#9841)
  Add network profile in Spring payload. (Azure#10313)
  Fix output-folder for go SDK in timeseriesinsight (Azure#10300)
  Alias Put Subscription API (Azure#10205)
  resource-resource-2019-03-01-python-config (Azure#10314)
  New Api version 2020-06-15- fixing certificate put/get issues with ARM  (Azure#10251)
  [DataFactory] Support source sink level staging (Azure#10290)
  Added swagger files for PostgreSQL Flexible Server preview (Azure#10020)
  [Hub Generated] Review request for Microsoft.AppPlatform to add version preview/2019-05-01-preview (Azure#10302)
  Adding Untracked Public Maintenance Config Resource Type (Azure#10281)
  add assignment resource id to the schema (Azure#10309)
  [datashare] sort api's and definitions (Azure#10273)
  Update readme.md all-api-versions tags (Azure#10296)
  Documentation updated (Azure#10283)
  [CosmosDB] Migration support of existing containers/databases to autoscale (Azure#9258)
  ...
00Kai0 pushed a commit to 00Kai0/azure-rest-api-specs that referenced this pull request Oct 12, 2020
… in artifact (Azure#10270)

* change response status code

* add activities

* fix ModelValidation and PrettierCheck

* long running operation

* fix description

* fix tab

Co-authored-by: Wan Yang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants