Reverting the schedule.json response code for Created to 201 as that … #3312

DeveloperTommy · 2018-06-27T22:01:34Z

…conforms with our Web Service API

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.

…conforms with our Web Service API

AutorestCI · 2018-06-27T22:02:47Z

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

AutorestCI · 2018-06-27T22:03:17Z

Automation for azure-sdk-for-go

Encountered a Subprocess error: (azure-sdk-for-go)

Command: ['/usr/local/bin/autorest', '/tmp/tmpg4f3glua/rest/specification/automation/resource-manager/readme.md', '--go', '--go-sdk-folder=/tmp/tmpg4f3glua/src/github.com/Azure/azure-sdk-for-go', '--multiapi', '--use=@microsoft.azure/autorest.go@~2.1.100', '--use-onever', '--verbose']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4280; node: v7.10.1]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/root/.autorest/@microsoft.azure_autorest-core@2.0.4280/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4280)
   Loading AutoRest extension '@microsoft.azure/autorest.go' (~2.1.100->2.1.104)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.38->2.3.38)
Processing batch task - {"tag":"package-2015-10"} .

ERROR (Fatal/DuplicateModelCollsion): Duplicated model name with non-identical definitions
    - file:///tmp/tmpg4f3glua/rest/specification/automation/resource-manager/Microsoft.Automation/stable/2015-10-31/runbook.json:1056:4 ($.definitions.JobStreamProperties)
    - file:///tmp/tmpg4f3glua/rest/specification/automation/resource-manager/Microsoft.Automation/stable/2015-10-31/job.json:780:4 ($.definitions.JobStreamProperties)
    - file:///tmp/tmpg4f3glua/rest/specification/automation/resource-manager/Microsoft.Automation/stable/2015-10-31/dscCompilationJob.json:561:4 ($.definitions.JobStreamProperties)
Process() cancelled due to exception : Cancellation requested.
Failure during batch task - {"tag":"package-2015-10"} -- Cancellation requested..
  Cancellation requested.

AutorestCI · 2018-06-27T22:03: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-06-27T22:04:10Z

Automation for azure-sdk-for-ruby

Encountered a Subprocess error: (azure-sdk-for-ruby)

Command: ['/usr/local/bin/autorest', '/tmp/tmp48zy5whu/rest/specification/automation/resource-manager/readme.md', '--multiapi', '--ruby', '--ruby-sdks-folder=/tmp/tmp48zy5whu/sdk', '--use=@microsoft.azure/autorest.ruby@3.0.20', '--version=preview']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4280; node: v7.10.1]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/root/.autorest/@microsoft.azure_autorest-core@2.0.4280/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4280)
   Loading AutoRest extension '@microsoft.azure/autorest.ruby' (3.0.20->3.0.20)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.1.22->2.1.22)
Processing batch task - {"tag":"package-2015-10"} .

ERROR (Fatal/DuplicateModelCollsion): Duplicated model name with non-identical definitions
    - file:///tmp/tmp48zy5whu/rest/specification/automation/resource-manager/Microsoft.Automation/stable/2015-10-31/runbook.json:1056:4 ($.definitions.JobStreamProperties)
    - file:///tmp/tmp48zy5whu/rest/specification/automation/resource-manager/Microsoft.Automation/stable/2015-10-31/job.json:780:4 ($.definitions.JobStreamProperties)
    - file:///tmp/tmp48zy5whu/rest/specification/automation/resource-manager/Microsoft.Automation/stable/2015-10-31/dscCompilationJob.json:561:4 ($.definitions.JobStreamProperties)
Process() cancelled due to exception : Cancellation requested.
Failure during batch task - {"tag":"package-2015-10"} -- Cancellation requested..
  Cancellation requested.

AutorestCI · 2018-06-27T22:07:20Z

Automation for azure-sdk-for-node

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

vrdmr

LGTM

annatisch · 2018-06-28T01:54:23Z

Thanks @DeveloperTommy!
It seems to be unrelated to the PR, but it looks like the spec has a duplicate model definition for "JobStreamProperties".
Would it be possible to correct this? (i.e. make all three definitions identical, or separate it out into a common definition referenced by all three)
As you can see - this is preventing the autorest generation of the Go and Ruby SDKs as configured by the current readme.

DeveloperTommy · 2018-06-28T02:00:38Z

Hi Anna,

Thanks for pointing that out. This change is somewhat urgent as it is blocking a recent large refactoring that we have done for our Swagger spec. My team does not own that asset; however, I'll note to our point of contact that we should fix this.

Thanks,
Tommy

vrdmr · 2018-06-28T06:34:05Z

Hi @annatisch - To resolve the "duplicate model definition for "JobStreamProperties"", I recently did the whole refactoring of our specs - #3190.

Would it be possible to correct this? (i.e. make all three definitions identical, or separate it out into a common definition referenced by all three)

If you go to out specs, you'll find common definitions for really common things and the individual definitions have been refactored.

As you can see - this is preventing the autorest generation of the Go and Ruby SDKs as configured by the current readme.

How should we change our readme.md to fix the issue with Go and Python?

annatisch · 2018-06-28T14:36:11Z

Thanks @vrdmr - I'm guessing in the earlier refactor the definitions for "JobStreamProperties" were probably only consolidated across particular API versions. However Go and Ruby will generate multiple-API-version-compatible SDKs.
This issue is with the 2015-10 API version (--tag=package-2015-10)
The package listing for 2015-10 will need to be either dropped from multi-API configuration (possibly breaking change for SDKs) or corrected (i.e. consistent JobStreamProperties between:

2015-10-31/runbook.json
2015-10-31/job.json
2015-10-31/dscCompilationJob

There are only two minor corrections that would be needed:

In dscCompilationJob.json the streamType enum is missing the debug option. If this is actually valid and debug should not be an option, it needs to be removed from job.json and runbook.json.
And finally runbook.json does not have the x-ms-client-flatten: true extension.

vrdmr · 2018-06-28T17:47:43Z

Thanks @annatisch. I'll take a look at the suggestions and do them. In the meantime, could you please approve this PR? This is urgent and is currently blocking our SDK release which we need for Powershell cmdlet work.

annatisch · 2018-06-28T18:12:49Z

@vrdmr - I can add the commit to this PR if you advise how to resolve the two conflicts identified above.
Would that be okay?

vrdmr · 2018-06-28T18:26:56Z

@annatisch - We wanted to make multiple changes - add a preview tag to Go and Python SDKs. And drop the tag 2015-10 as this is not being used and we have moved to the 2018 tag.

lmazuel · 2018-06-28T19:09:22Z

@vrdmr could you define more precisely "this is not being used". Do you say the endpoint does not exist anymore and is not reachable? Or do you say you don't recommend it anymore?
If the end-point is still reachable, do you have metrics that tells you it's not used anymore by no-one on earth?

The contract with multi-api versioning packages in Go, Ruby and Python (and soon more language by the way), is that we ship all Swagger that matches a real end-point. Meaning customers can decide without installing an old version of SDK what Api version they want. Not shipping an api-version already shipped is then not an option, since it might break customers.

If we merge this PR as-is, this means that you're telling Go/Ruby/Python SDK customers that they can't get the content of this PR and all latest fixes and updates, because you prioritized Powershell users. Being that the change needed is faster to implement that the time we spent to discuss it, I think we should consider making everybody happy and not just Powershell :)

Edit: My biggest fear when a sentence contains "I'll do it later" is that it becomes "never" because we all have busy life and work. If you commit to fix old api-versions asap after this PR is released, I think Go/Ruby/Python can wait a few days. It's not that critical to release everything the exact same day if the waiting gap is short. Adding @jhendrixMSFT for Go and @sarangan12 for Ruby FYI that Go/Ruby might be not in sync with C#/Powershell once this is merged.

lmazuel · 2018-06-28T22:26:31Z

@annatisch I discussed with @vrdmr off-Github, please merge the PR without considering the Autorest failure for now to unlock him. The fix PR should be here by next Monday.
@vrdmr in your future PR, please mention me and @annatisch in the text for context.
Thanks!

Reverting the schedule.json response code for Created to 201 as that …

0430735

…conforms with our Web Service API

azuresdkci assigned annatisch Jun 27, 2018

azuresdkci requested a review from annatisch June 27, 2018 22:02

AutorestCI mentioned this pull request Jun 27, 2018

[AutoPR automation/resource-manager] Reverting the schedule.json response code for Created to 201 as that … Azure/azure-sdk-for-python#2822

Merged

AutorestCI mentioned this pull request Jun 27, 2018

[AutoPR automation/resource-manager] Reverting the schedule.json response code for Created to 201 as that … Azure/azure-sdk-for-node#3075

Merged

DeveloperTommy added 2 commits June 27, 2018 15:14

Also added dummy 200 response for idempotency

2e54eec

Added an example for 200 as well

e49e20d

vrdmr approved these changes Jun 27, 2018

View reviewed changes

annatisch approved these changes Jun 28, 2018

View reviewed changes

annatisch merged commit 8068e2a into Azure:master Jun 28, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reverting the schedule.json response code for Created to 201 as that … #3312

Reverting the schedule.json response code for Created to 201 as that … #3312

DeveloperTommy commented Jun 27, 2018 •

edited by azuresdkci

Loading

AutorestCI commented Jun 27, 2018 •

edited

Loading

AutorestCI commented Jun 27, 2018 •

edited

Loading

AutorestCI commented Jun 27, 2018 •

edited

Loading

AutorestCI commented Jun 27, 2018 •

edited

Loading

AutorestCI commented Jun 27, 2018 •

edited

Loading

vrdmr left a comment

annatisch commented Jun 28, 2018

DeveloperTommy commented Jun 28, 2018 •

edited

Loading

vrdmr commented Jun 28, 2018 •

edited

Loading

annatisch commented Jun 28, 2018

vrdmr commented Jun 28, 2018

annatisch commented Jun 28, 2018

vrdmr commented Jun 28, 2018

lmazuel commented Jun 28, 2018 •

edited

Loading

lmazuel commented Jun 28, 2018

Reverting the schedule.json response code for Created to 201 as that … #3312

Reverting the schedule.json response code for Created to 201 as that … #3312

Conversation

DeveloperTommy commented Jun 27, 2018 • edited by azuresdkci Loading

PR information

Quality of Swagger

AutorestCI commented Jun 27, 2018 • edited Loading

Automation for azure-sdk-for-java

AutorestCI commented Jun 27, 2018 • edited Loading

Automation for azure-sdk-for-go

AutorestCI commented Jun 27, 2018 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Jun 27, 2018 • edited Loading

Automation for azure-sdk-for-ruby

AutorestCI commented Jun 27, 2018 • edited Loading

Automation for azure-sdk-for-node

vrdmr left a comment

Choose a reason for hiding this comment

annatisch commented Jun 28, 2018

DeveloperTommy commented Jun 28, 2018 • edited Loading

vrdmr commented Jun 28, 2018 • edited Loading

annatisch commented Jun 28, 2018

vrdmr commented Jun 28, 2018

annatisch commented Jun 28, 2018

vrdmr commented Jun 28, 2018

lmazuel commented Jun 28, 2018 • edited Loading

lmazuel commented Jun 28, 2018

DeveloperTommy commented Jun 27, 2018 •

edited by azuresdkci

Loading

AutorestCI commented Jun 27, 2018 •

edited

Loading

AutorestCI commented Jun 27, 2018 •

edited

Loading

AutorestCI commented Jun 27, 2018 •

edited

Loading

AutorestCI commented Jun 27, 2018 •

edited

Loading

AutorestCI commented Jun 27, 2018 •

edited

Loading

DeveloperTommy commented Jun 28, 2018 •

edited

Loading

vrdmr commented Jun 28, 2018 •

edited

Loading

lmazuel commented Jun 28, 2018 •

edited

Loading