Skip to content

Conversation

@khughes66
Copy link

@khughes66 khughes66 commented Jun 28, 2018

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

@AutorestCI
Copy link

AutorestCI commented Jun 28, 2018

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@AutorestCI
Copy link

AutorestCI commented Jun 28, 2018

Automation for azure-sdk-for-python

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

Command: ['/usr/local/bin/autorest', '/tmp/tmp38mkjt8j/rest/specification/automation/resource-manager/readme.md', '--multiapi', '--python', '--python-mode=update', '--python-sdks-folder=/tmp/tmp38mkjt8j/sdk', '[email protected]/autorest.python@~3.0.56', '--version=preview']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4283; node: v8.11.3]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4283)
   Loading AutoRest extension '@microsoft.azure/autorest.python' (~3.0.56->3.0.58)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.44->2.3.44)

ERROR (Fatal/DuplicateModelCollsion): Duplicated model name with non-identical definitions
    - file:///tmp/tmp38mkjt8j/rest/specification/automation/resource-manager/Microsoft.Automation/stable/2018-06-30/runbook.json:1056:4 ($.definitions.JobStreamProperties)
    - file:///tmp/tmp38mkjt8j/rest/specification/automation/resource-manager/Microsoft.Automation/stable/2018-01-15/dscCompilationJob.json:546:4 ($.definitions.JobStreamProperties)
    - file:///tmp/tmp38mkjt8j/rest/specification/automation/resource-manager/Microsoft.Automation/preview/2017-05-15-preview/job.json:627:4 ($.definitions.JobStreamProperties)
Process() cancelled due to exception : Cancellation requested.
  Cancellation requested.

@AutorestCI
Copy link

AutorestCI commented Jun 28, 2018

Automation for azure-sdk-for-go

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

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

AutoRest code generation utility [version: 2.0.4283; node: v8.11.3]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4283)
   Loading AutoRest extension '@microsoft.azure/autorest.go' (~2.1.110->2.1.110)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.38->2.3.38)
Processing batch task - {"tag":"package-2015-10"} .
Processing batch task - {"tag":"package-2017-05-preview"} .
Processing batch task - {"tag":"package-2018-01-preview"} .
Processing batch task - {"tag":"package-2018-06-preview"} .

ERROR (Fatal/DuplicateModelCollsion): Duplicated model name with non-identical definitions
    - file:///tmp/tmp_co_y2qj/rest/specification/automation/resource-manager/Microsoft.Automation/stable/2018-06-30/runbook.json:1056:4 ($.definitions.JobStreamProperties)
    - file:///tmp/tmp_co_y2qj/rest/specification/automation/resource-manager/Microsoft.Automation/stable/2018-01-15/dscCompilationJob.json:546:4 ($.definitions.JobStreamProperties)
    - file:///tmp/tmp_co_y2qj/rest/specification/automation/resource-manager/Microsoft.Automation/preview/2017-05-15-preview/job.json:627:4 ($.definitions.JobStreamProperties)
Process() cancelled due to exception : Cancellation requested.
Failure during batch task - {"tag":"package-2018-06-preview"} -- Cancellation requested..
  Cancellation requested.

@AutorestCI
Copy link

AutorestCI commented Jun 28, 2018

Automation for azure-sdk-for-ruby

A PR has been created for you:
Azure/azure-sdk-for-ruby#1578

@AutorestCI
Copy link

AutorestCI commented Jun 28, 2018

Automation for azure-sdk-for-node

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

Command: ['/usr/local/bin/autorest', '/tmp/tmp7yh6v7f1/rest/specification/automation/resource-manager/readme.md', '--license-header=MICROSOFT_MIT_NO_VERSION', '--node-sdks-folder=/tmp/tmp7yh6v7f1/sdk', '--nodejs', '[email protected]/autorest.nodejs@^2.1.43']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4283; node: v8.11.3]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4283)
   Loading AutoRest extension '@microsoft.azure/autorest.nodejs' (^2.1.43->2.1.79)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.44->2.3.44)

ERROR (Fatal/DuplicateModelCollsion): Duplicated model name with non-identical definitions
    - file:///tmp/tmp7yh6v7f1/rest/specification/automation/resource-manager/Microsoft.Automation/stable/2018-06-30/runbook.json:1056:4 ($.definitions.JobStreamProperties)
    - file:///tmp/tmp7yh6v7f1/rest/specification/automation/resource-manager/Microsoft.Automation/stable/2018-01-15/dscCompilationJob.json:546:4 ($.definitions.JobStreamProperties)
    - file:///tmp/tmp7yh6v7f1/rest/specification/automation/resource-manager/Microsoft.Automation/preview/2017-05-15-preview/job.json:627:4 ($.definitions.JobStreamProperties)
Process() cancelled due to exception : Cancellation requested.
  Cancellation requested.

@khughes66
Copy link
Author

Martin, can you please review? Thanks.

@khughes66
Copy link
Author

@marstr Please review

@marstr marstr added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Jul 2, 2018
"modelAsString": true
}
},
"x-ms-parameter-location": "method"
Copy link
Member

Choose a reason for hiding this comment

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

Heads up, I believe that will make this a breaking change inside of an API Version marked stable.

Copy link
Member

Choose a reason for hiding this comment

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

Hi @marstr - The SDK hasn't been released and we are correcting this parameter as this should be a method level parameter for the NodeCountInformation call.

],
"x-ms-enum": {
"name": "HttpStatusCode",
"modelAsString": false
Copy link
Member

Choose a reason for hiding this comment

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

We recommend using "modelAsString": true to prevent breaking changes in future if you need to add a supported value.

@vrdmr
Copy link
Member

vrdmr commented Jul 10, 2018

@ravbhatnagar Could you please take a look at the PR.

Copy link
Member

@marstr marstr left a comment

Choose a reason for hiding this comment

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

Looks like both some linter and model errors have popped up, and there are now conflicts with the master branch. Please take a look here:
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/401942873#L775

@vrdmr
Copy link
Member

vrdmr commented Jul 13, 2018

@khughes66 - Can you check JobStreamProperties in the runbook, and make sure they are same - Travis Model issues.

@ravbhatnagar
Copy link
Contributor

Looks good. The change is to expose publish action on runbooks instead of draft. This change was discussed in person with ARM.

@ravbhatnagar ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Jul 20, 2018
@marstr
Copy link
Member

marstr commented Jul 25, 2018

I'm going to be on vacation until Monday, July 30th. If this PR needs attention in the interim, please contact my boss, @salameer.

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@vrdmr
Copy link
Member

vrdmr commented Aug 8, 2018

@marstr When running the autorest --python locally, it does not throw the duplicate definitions for JobStreamProperties error. Could you please take a look at this issue?

varads-mbp:resource-manager varadmeru$ autorest readme.md --python --output-folder="."
AutoRest code generation utility [version: 2.0.4283; node: v10.3.0]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/Users/varadmeru/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4283)
   Loading AutoRest extension '@microsoft.azure/autorest.python' (~2.1.26->2.1.40)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.44->2.3.44)
varads-mbp:resource-manager varadmeru$ 

@marstr
Copy link
Member

marstr commented Aug 9, 2018

Looking at the build logs here: https://travis-ci.org/Azure/azure-rest-api-specs/jobs/413755232#L613 it says that there are duplicated definitions of JobStreamProperties, which conflict. Given that they're in separate API Versions, 2018-01-15 and 2017-05-15-preview.

In the command you executed above, you didn't provide the flag --multiapi, which means AutoRest will only process the one default tag in the readme. I suspect there's a tag in README.md which accidentally includes both specification/automation/resource-manager/Microsoft.Automation/stable/2018-01-15/dscCompilationJob.json and specification/automation/resource-manager/Microsoft.Automation/preview/2017-05-15-preview/job.json.

Let me know what you find!

@vrdmr
Copy link
Member

vrdmr commented Aug 15, 2018

@khughes66 Could you please close this PR. #3646 fixes the issues and passes.

@khughes66
Copy link
Author

There were some strange issues with this PR (see the checks above). We duplicated it into PR #3646, which has passed the checks. Closing this PR in favor of that one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants