Remove 4xx responses and change schema to file #3363

vrdmr · 2018-07-05T23:39:10Z

Removing the 4xx response codes and changing the schema from string to file to unblock the SDK serialization issue.
FYI: @najams @shahabhijeet

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-07-05T23:40:17Z

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#1959

AutorestCI · 2018-07-05T23:40:20Z

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

AutorestCI · 2018-07-05T23:41:53Z

Automation for azure-sdk-for-ruby

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-ruby#1476

AutorestCI · 2018-07-05T23:44:23Z

Automation for azure-sdk-for-node

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-node#2653

AutorestCI · 2018-07-05T23:46:21Z

Automation for azure-sdk-for-go

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-go#2220

hovsepm · 2018-07-06T21:37:19Z

@vrdmr could you provide some context on what does it mean "unblock the SDK serialization issue."? As for current status of your PR - you are introducing breaking changes to stable API version.

jhendrixMSFT · 2018-07-09T16:25:48Z

@AutorestCI regenerate azure-sdk-for-go

vrdmr · 2018-07-09T20:04:31Z

@hovsepm The serialization issue in dotnet SDK is mentioned in details here (slack link), but tl;dr is that Autorest generates code which expects the output schema to be json and we return file. This causes a serialization issues and is breaking out current SDK users (our SDK is still in the preview though).

Thus, for runbooks resource, I am reverting it to be back to where it was, before #2466. After discussing with @shahabhijeet, he and I thought it would be best to get the spec back to its original shape.

hovsepm · 2018-07-09T21:00:44Z

@vrdmr the issue you pointed out in Slack channel is about "2017-05-15-preview" version - why do you have breaking changes in version 2015-10-31 which is marked as stable and I assume was shipped?

vrdmr · 2018-07-09T23:46:40Z

@hovsepm The #2466 PR made the changes in bot 2015-10-31 (Runbooks) and 2017-05-15-preview (Jobs) resources. For the GetContent call in the runbooks.json resource, the change was added in #2466 (Link to Blame).

vrdmr · 2018-07-11T19:16:14Z

Ping @hovsepm.

hovsepm · 2018-07-11T20:49:46Z

@vrdmr I'm asking about service response changes - in many places your change removes 409 return codes. These are new breaking changes in the existing stable API version.

vrdmr · 2018-07-11T21:04:49Z

@hovsepm Yes, we are removing 409 here and other places as in the SDKs, they are being returned as successful and not generating exceptions, which they should. This was caught by @dsgouda in the AzureSDK PR. We do want exceptions being thrown for an error case, whereas currently, it would not raise for any.

hovsepm · 2018-07-11T21:11:01Z

@vrdmr you do realize that there are many applications that are already shipped using API version 2015-10-31 that do use 409 as return code and have some logic to handle that return codes. Changing it to throw an exception will cause breaks for any patch in the code that will use latest generated SDK. I'd strongly suggest to change return types to exception only in the new/preview API version.

vrdmr · 2018-07-11T23:53:42Z

@hovsepm Updated the Spec. Could you please review and merge this PR.

Thanks,
Varad

hovsepm · 2018-07-12T00:03:18Z

@vrdmr please fix model validation errors:

[]
  id: 'OAV108',
  message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Get Job Output" in operation "Job_GetOutput".:\n',
  innerErrors: 
   [ { code: 'INVALID_RESPONSE_BODY',
       errors: 
        [ { code: 'INVALID_TYPE',
            params: [ 'file', 'string' ],
            message: 'Expected type file but found type string',
            path: [] } ],
       message: 'Invalid body: Expected type file but found type string',
       path: [] } ],
  level: '\u001b[31merror\u001b[39m',
  [Symbol(level)]: 'error',
  [Symbol(splat)]: 
   [ { code: 'RESPONSE_VALIDATION_ERROR',
       id: 'OAV108',
       message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Get Job Output" in operation "Job_GetOutput".',
       innerErrors: 
        [ { code: 'INVALID_RESPONSE_BODY',
            errors: 
             [ { code: 'INVALID_TYPE',
                 params: [ 'file', 'string' ],
                 message: 'Expected type file but found type string',
                 path: [] } ],
            message: 'Invalid body: Expected type file but found type string',
            path: [] } ] } ],
  [Symbol(message)]: '{"code":"RESPONSE_VALIDATION_ERROR","id":"OAV108","message":"Found errors in validating the response with statusCode \\"200\\" for x-ms-example \\"Get Job Output\\" in operation \\"Job_GetOutput\\".:\\n","innerErrors":[{"code":"INVALID_RESPONSE_BODY","errors":[{"code":"INVALID_TYPE","params":["file","string"],"message":"Expected type file but found type string","path":[]}],"message":"Invalid body: Expected type file but found type string","path":[]}],"level":"error"}' }
{ code: 'RESPONSE_VALIDATION_ERROR',
  id: 'OAV108',
  message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Get Job Runbook Content" in operation "Job_GetRunbookContent".:\n',
  innerErrors: 
   [ { code: 'INVALID_CONTENT_TYPE',
       message: 'Invalid Content-Type (text/powershell).  These are supported: application/json',
       path: [] },
     { code: 'INVALID_RESPONSE_BODY',
       errors: 
        [ { code: 'INVALID_TYPE',
            params: [ 'file', 'string' ],
            message: 'Expected type file but found type string',
            path: [] } ],
       message: 'Invalid body: Expected type file but found type string',
       path: [] } ],
  level: '\u001b[31merror\u001b[39m',
  [Symbol(level)]: 'error',
  [Symbol(splat)]: 
   [ { code: 'RESPONSE_VALIDATION_ERROR',
       id: 'OAV108',
       message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Get Job Runbook Content" in operation "Job_GetRunbookContent".',
       innerErrors: 
        [ { code: 'INVALID_CONTENT_TYPE',
            message: 'Invalid Content-Type (text/powershell).  These are supported: application/json',
            path: [] },
          { code: 'INVALID_RESPONSE_BODY',
            errors: 
             [ { code: 'INVALID_TYPE',
                 params: [ 'file', 'string' ],
                 message: 'Expected type file but found type string',
                 path: [] } ],
            message: 'Invalid body: Expected type file but found type string',
            path: [] } ] } ],
  [Symbol(message)]: '{"code":"RESPONSE_VALIDATION_ERROR","id":"OAV108","message":"Found errors in validating the response with statusCode \\"200\\" for x-ms-example \\"Get Job Runbook Content\\" in operation \\"Job_GetRunbookContent\\".:\\n","innerErrors":[{"code":"INVALID_CONTENT_TYPE","message":"Invalid Content-Type (text/powershell).  These are supported: application/json","path":[]},{"code":"INVALID_RESPONSE_BODY","errors":[{"code":"INVALID_TYPE","params":["file","string"],"message":"Expected type file but found type string","path":[]}],"message":"Invalid body: Expected type file but found type string","path":[]}],"level":"error"}' }
{ AssertionError [ERR_ASSERTION]: swagger "specification/automation/resource-manager/Microsoft.Automation/preview/2017-05-15-preview/job.json" contains model validation errors.
    at Context.<anonymous> (/home/travis/build/Azure/azure-rest-api-specs/test/model.js:20:16)
    at <anonymous>
  generatedMessage: false,
  name: 'AssertionError [ERR_ASSERTION]',
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '==' }
{ code: 'RESPONSE_VALIDATION_ERROR',
  id: 'OAV108',
  message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Get runbook draft content" in operation "RunbookDraft_GetContent".:\n',
  innerErrors: 
   [ { code: 'INVALID_RESPONSE_BODY',
       errors: 
        [ { code: 'INVALID_TYPE',
            params: [ 'file', 'string' ],
            message: 'Expected type file but found type string',
            path: [] } ],
       message: 'Invalid body: Expected type file but found type string',
       path: [] } ],
  level: '\u001b[31merror\u001b[39m',
  [Symbol(level)]: 'error',
  [Symbol(splat)]: 
   [ { code: 'RESPONSE_VALIDATION_ERROR',
       id: 'OAV108',
       message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Get runbook draft content" in operation "RunbookDraft_GetContent".',
       innerErrors: 
        [ { code: 'INVALID_RESPONSE_BODY',
            errors: 
             [ { code: 'INVALID_TYPE',
                 params: [ 'file', 'string' ],
                 message: 'Expected type file but found type string',
                 path: [] } ],
            message: 'Invalid body: Expected type file but found type string',
            path: [] } ] } ],
  [Symbol(message)]: '{"code":"RESPONSE_VALIDATION_ERROR","id":"OAV108","message":"Found errors in validating the response with statusCode \\"200\\" for x-ms-example \\"Get runbook draft content\\" in operation \\"RunbookDraft_GetContent\\".:\\n","innerErrors":[{"code":"INVALID_RESPONSE_BODY","errors":[{"code":"INVALID_TYPE","params":["file","string"],"message":"Expected type file but found type string","path":[]}],"message":"Invalid body: Expected type file but found type string","path":[]}],"level":"error"}' }
{ code: 'RESPONSE_STATUS_CODE_NOT_IN_EXAMPLE',
  id: 'OAV111',
  message: 'Following response status codes "200" for operation "RunbookDraft_ReplaceContent" were present in the swagger spec, however they were not present in x-ms-examples. Please provide them.',
  innerErrors: undefined,
  level: '\u001b[31merror\u001b[39m',
  [Symbol(level)]: 'error',
  [Symbol(message)]: '{"code":"RESPONSE_STATUS_CODE_NOT_IN_EXAMPLE","id":"OAV111","message":"Following response status codes \\"200\\" for operation \\"RunbookDraft_ReplaceContent\\" were present in the swagger spec, however they were not present in x-ms-examples. Please provide them.","level":"error"}' }
{ code: 'RESPONSE_VALIDATION_ERROR',
  id: 'OAV108',
  message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Create or update runbook draft" in operation "RunbookDraft_ReplaceContent".:\n',
  innerErrors: 
   [ { code: 'RESPONSE_STATUS_CODE_NOT_IN_EXAMPLE',
       id: 'OAV111',
       message: 'Following response status codes "200" for operation "RunbookDraft_ReplaceContent" were present in the swagger spec, however they were not present in x-ms-examples. Please provide them.',
       innerErrors: undefined,
       level: 'error',
       [Symbol(level)]: 'error',
       [Symbol(message)]: '{"code":"RESPONSE_STATUS_CODE_NOT_IN_EXAMPLE","id":"OAV111","message":"Following response status codes \\"200\\" for operation \\"RunbookDraft_ReplaceContent\\" were present in the swagger spec, however they were not present in x-ms-examples. Please provide them.","level":"error"}' } ],
  level: '\u001b[31merror\u001b[39m',
  [Symbol(level)]: 'error',
  [Symbol(splat)]: 
   [ { code: 'RESPONSE_VALIDATION_ERROR',
       id: 'OAV108',
       message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Create or update runbook draft" in operation "RunbookDraft_ReplaceContent".',
       innerErrors: 
        [ { code: 'RESPONSE_STATUS_CODE_NOT_IN_EXAMPLE',
            id: 'OAV111',
            message: 'Following response status codes "200" for operation "RunbookDraft_ReplaceContent" were present in the swagger spec, however they were not present in x-ms-examples. Please provide them.',
            innerErrors: undefined,
            level: 'error',
            [Symbol(level)]: 'error',
            [Symbol(message)]: '{"code":"RESPONSE_STATUS_CODE_NOT_IN_EXAMPLE","id":"OAV111","message":"Following response status codes \\"200\\" for operation \\"RunbookDraft_ReplaceContent\\" were present in the swagger spec, however they were not present in x-ms-examples. Please provide them.","level":"error"}' } ] } ],
  [Symbol(message)]: '{"code":"RESPONSE_VALIDATION_ERROR","id":"OAV108","message":"Found errors in validating the response with statusCode \\"200\\" for x-ms-example \\"Create or update runbook draft\\" in operation \\"RunbookDraft_ReplaceContent\\".:\\n","innerErrors":[{"code":"RESPONSE_STATUS_CODE_NOT_IN_EXAMPLE","id":"OAV111","message":"Following response status codes \\"200\\" for operation \\"RunbookDraft_ReplaceContent\\" were present in the swagger spec, however they were not present in x-ms-examples. Please provide them.","level":"error"}],"level":"error"}' }
{ code: 'RESPONSE_VALIDATION_ERROR',
  id: 'OAV108',
  message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Get runbook content" in operation "Runbook_GetContent".:\n',
  innerErrors: 
   [ { code: 'INVALID_RESPONSE_BODY',
       errors: 
        [ { code: 'INVALID_TYPE',
            params: [ 'file', 'string' ],
            message: 'Expected type file but found type string',
            path: [] } ],
       message: 'Invalid body: Expected type file but found type string',
       path: [] } ],
  level: '\u001b[31merror\u001b[39m',
  [Symbol(level)]: 'error',
  [Symbol(splat)]: 
   [ { code: 'RESPONSE_VALIDATION_ERROR',
       id: 'OAV108',
       message: 'Found errors in validating the response with statusCode "200" for x-ms-example "Get runbook content" in operation "Runbook_GetContent".',
       innerErrors: 
        [ { code: 'INVALID_RESPONSE_BODY',
            errors: 
             [ { code: 'INVALID_TYPE',
                 params: [ 'file', 'string' ],
                 message: 'Expected type file but found type string',
                 path: [] } ],
            message: 'Invalid body: Expected type file but found type string',
            path: [] } ] } ],
  [Symbol(message)]: '{"code":"RESPONSE_VALIDATION_ERROR","id":"OAV108","message":"Found errors in validating the response with statusCode \\"200\\" for x-ms-example \\"Get runbook content\\" in operation \\"Runbook_GetContent\\".:\\n","innerErrors":[{"code":"INVALID_RESPONSE_BODY","errors":[{"code":"INVALID_TYPE","params":["file","string"],"message":"Expected type file but found type string","path":[]}],"message":"Invalid body: Expected type file but found type string","path":[]}],"level":"error"}' }

hovsepm · 2018-07-12T00:10:31Z

@vrdmr ignore the error.

nit fixes: Remove 4xx responses and schema to file

e8a0ed1

azuresdkci assigned hovsepm Jul 5, 2018

azuresdkci requested a review from hovsepm July 5, 2018 23:39

AutorestCI mentioned this pull request Jul 5, 2018

[AutoPR automation/resource-manager] Remove 4xx responses and change schema to file Azure/azure-sdk-for-python#2881

Merged

AutorestCI mentioned this pull request Jul 5, 2018

[AutoPR automation/resource-manager] Remove 4xx responses and change schema to file Azure/azure-sdk-for-ruby#1446

Merged

AutorestCI mentioned this pull request Jul 5, 2018

[AutoPR automation/resource-manager] Remove 4xx responses and change schema to file Azure/azure-sdk-for-node#3124

Merged

AutorestCI mentioned this pull request Jul 5, 2018

[AutoPR automation/resource-manager] Remove 4xx responses and change schema to file Azure/azure-sdk-for-go#2192

Closed

hovsepm added the potential-sdk-breaking-change label Jul 6, 2018

AutorestCI mentioned this pull request Jul 9, 2018

[AutoPR automation/resource-manager] Remove 4xx responses and change schema to file Azure/azure-sdk-for-go#2200

Merged

Reverting changes in stable APIs

8732be5

hovsepm approved these changes Jul 12, 2018

View reviewed changes

hovsepm merged commit 8f83ec5 into Azure:master Jul 12, 2018

vrdmr deleted the vameru-minorfixes-automationspec branch July 16, 2018 18:22

This was referenced Jul 16, 2018

Changing schema from string to file #3420

Merged

New SDK update for Automation Azure/azure-sdk-for-net#4570

Closed

vrdmr mentioned this pull request Jul 24, 2018

New SDK update for Automation - 3.2.0-preview Azure/azure-sdk-for-net#4583

Merged

9 tasks

vrdmr mentioned this pull request Aug 28, 2018

Fixing the spec for DSCConfigurations. #3749

Merged

10 tasks

vrdmr mentioned this pull request Oct 3, 2018

Fix the runbook resource schema issues #4017

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove 4xx responses and change schema to file #3363

Remove 4xx responses and change schema to file #3363

vrdmr commented Jul 5, 2018 •

edited

Loading

AutorestCI commented Jul 5, 2018 •

edited

Loading

AutorestCI commented Jul 5, 2018 •

edited

Loading

AutorestCI commented Jul 5, 2018 •

edited

Loading

AutorestCI commented Jul 5, 2018 •

edited

Loading

AutorestCI commented Jul 5, 2018 •

edited

Loading

hovsepm commented Jul 6, 2018

jhendrixMSFT commented Jul 9, 2018

vrdmr commented Jul 9, 2018

hovsepm commented Jul 9, 2018

vrdmr commented Jul 9, 2018

vrdmr commented Jul 11, 2018

hovsepm commented Jul 11, 2018

vrdmr commented Jul 11, 2018

hovsepm commented Jul 11, 2018

vrdmr commented Jul 11, 2018

hovsepm commented Jul 12, 2018

hovsepm commented Jul 12, 2018

Remove 4xx responses and change schema to file #3363

Remove 4xx responses and change schema to file #3363

Conversation

vrdmr commented Jul 5, 2018 • edited Loading

PR information

Quality of Swagger

AutorestCI commented Jul 5, 2018 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Jul 5, 2018 • edited Loading

Automation for azure-sdk-for-java

AutorestCI commented Jul 5, 2018 • edited Loading

Automation for azure-sdk-for-ruby

AutorestCI commented Jul 5, 2018 • edited Loading

Automation for azure-sdk-for-node

AutorestCI commented Jul 5, 2018 • edited Loading

Automation for azure-sdk-for-go

hovsepm commented Jul 6, 2018

jhendrixMSFT commented Jul 9, 2018

vrdmr commented Jul 9, 2018

hovsepm commented Jul 9, 2018

vrdmr commented Jul 9, 2018

vrdmr commented Jul 11, 2018

hovsepm commented Jul 11, 2018

vrdmr commented Jul 11, 2018

hovsepm commented Jul 11, 2018

vrdmr commented Jul 11, 2018

hovsepm commented Jul 12, 2018

hovsepm commented Jul 12, 2018

vrdmr commented Jul 5, 2018 •

edited

Loading

AutorestCI commented Jul 5, 2018 •

edited

Loading

AutorestCI commented Jul 5, 2018 •

edited

Loading

AutorestCI commented Jul 5, 2018 •

edited

Loading

AutorestCI commented Jul 5, 2018 •

edited

Loading

AutorestCI commented Jul 5, 2018 •

edited

Loading