-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
[Hub Generated] Review request for Microsoft.WindowsESU to add version preview/2019-09-16-preview #7888
[Hub Generated] Review request for Microsoft.WindowsESU to add version preview/2019-09-16-preview #7888
Conversation
Automation for azure-sdk-for-pythonA PR has been created for you: |
Automation for azure-sdk-for-goEncountered a Subprocess error: (azure-sdk-for-go)
Command: ['/usr/local/bin/autorest', '/tmp/tmpzbe6m3kq/rest/specification/windowsesu/resource-manager/readme.md', '--go', '--go-sdk-folder=/tmp/tmpzbe6m3kq/src/github.com/Azure/azure-sdk-for-go', '--multiapi', '--preview-chk', '[email protected]/autorest.go@~2.1.137', '--use-onever', '--verbose'] AutoRest code generation utility [version: 2.0.4283; node: v10.15.3]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
There is a new version of AutoRest available (2.0.4407).
> You can install the newer version with with npm install -g autorest@latest
Loading AutoRest core '/root/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4407)
Including configuration file 'file:///tmp/tmpzbe6m3kq/rest/specification/windowsesu/resource-manager/readme.go.md'
Loading AutoRest extension '@microsoft.azure/autorest.go' (~2.1.137->2.1.137)
Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.38->2.3.38)
FATAL: System.InvalidOperationException: codegen for preview swagger Microsoft.WindowsESU/preview/2019-09-16-preview/windowsesu.json must be under a preview subdirectory
at AutoRest.Go.CodeGeneratorGo.<Generate>d__7.MoveNext() in /home/vsts/work/1/s/src/CodeGeneratorGo.cs:line 56
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at AutoRest.Go.Program.<ProcessInternal>d__3.MoveNext() in /home/vsts/work/1/s/src/Program.cs:line 107
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
at NewPlugin.<Process>d__20.MoveNext() in /home/vsts/work/1/s/autorest.common/src/Plugins/NewPlugin.cs:line 163
FATAL: go/generate - FAILED
FATAL: Error: Plugin go reported failure.
Process() cancelled due to exception : Plugin go reported failure.
Error: Plugin go reported failure. |
The spec was already approved in the private repo: |
Azure Pipelines successfully started running 1 pipeline(s). |
azure-sdk-for-python - Release⌛ Pending...
|
azure-sdk-for-java - Release⌛ Pending...
|
azure-sdk-for-go - Release⌛ Pending...
|
azure-sdk-for-js - Release⌛ Pending...
|
azure-sdk-for-net - Release⌛ Pending...
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
Azure Pipelines successfully started running 1 pipeline(s). |
@NullMDR can you take a look at the Go SDK automation failure? |
@ArcturusZhang Could you please take a look at the go generation failure? Seems like the autorest.go has some problem. |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Signing off with a few comments
"location" | ||
], | ||
"properties": { | ||
"id": { |
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.
It's best to carve the common resource properties into a shared definition. Better yet, use an allOf reference to link the common definition from common-types.
azure-rest-api-specs/specification/common-types/resource-management/v1/types.json
Line 45 in a6f41b5
"TrackedResource": { |
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've added references to common types
}, | ||
"installedServerNumber": { | ||
"description": "Number of activations/servers using the MAK key.", | ||
"type": "integer", |
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 user set or read-only?
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.
Set by user on creation, immutable, as most of other properties.
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.
LGTM for the go SDK part
Azure Pipelines successfully started running 1 pipeline(s). |
Can one of the admins verify this patch? |
If you are a MSFT employee you can view your work branch via this link.
Contribution checklist: