Introduce Azure Traffic Manager 2018-04-01 API version with new features#3162
Introduce Azure Traffic Manager 2018-04-01 API version with new features#3162ttlcgl wants to merge 4 commits intoAzure:masterfrom ttlcgl:master
Conversation
Automation for azure-sdk-for-pythonA PR has been created for you: |
Automation for azure-sdk-for-nodeA PR has been created for you based on this PR content. Once this PR will be merged, content will be added to your service PR: |
Automation for azure-libraries-for-javaA PR has been created for you based on this PR content. Once this PR will be merged, content will be added to your service PR: |
sergey-shandar
added
the
WaitForARMFeedback
|
@ravbhatnagar new API version for traffic manager. |
Automation for azure-sdk-for-rubyEncountered a Subprocess error: (azure-sdk-for-ruby)
Command: ['/usr/local/bin/autorest', '/tmp/tmpssly65g4/rest/specification/trafficmanager/resource-manager/readme.md', '--multiapi', '--ruby', '--ruby-sdks-folder=/tmp/tmpssly65g4/sdk', '--use=@microsoft.azure/autorest.ruby@3.0.20', '--version=preview'] AutoRest code generation utility [version: 2.0.4280; node: v8.11.3]
(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-11"} .
Processing batch task - {"tag":"package-2017-03"} .
Processing batch task - {"tag":"package-2017-05"} .
Processing batch task - {"tag":"package-2017-09-preview-only"} .
Processing batch task - {"tag":"package-2018-02"} .
FATAL: System.NullReferenceException: CodeModel doesn't have api-version set, specs in code model have mismatching api-versions.
at AutoRest.Ruby.Azure.CodeGeneratorRba.<Generate>d__6.MoveNext() in C:\work\oneautorest\autorest.ruby\src\azure\CodeGeneratorRba.cs:line 52
--- 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.Ruby.Program.<ProcessInternal>d__3.MoveNext() in C:\work\oneautorest\autorest.ruby\src\Program.cs:line 112
--- 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 NewPlugin.<Process>d__15.MoveNext()
FATAL: ruby/generate - FAILED
FATAL: Error: Plugin ruby reported failure.
Process() cancelled due to exception : Plugin ruby reported failure.
Failure during batch task - {"tag":"package-2018-02"} -- Error: Plugin ruby reported failure..
Error: Plugin ruby reported failure. |
Automation for azure-sdk-for-goNothing to generate for azure-sdk-for-go |
dsgouda
left a comment
dsgouda
left a comment
There was a problem hiding this comment.
Partial review since @sergey-shandar will be taking over this one
| "profileName": "azsmnet6386", | ||
| "endpointType": "ExternalEndpoints", | ||
| "endpointName": "azsmnet7187", | ||
| "api-version": "2017-05-01" |
There was a problem hiding this comment.
I think we should verify whether the versions mentioned in these examples match with the version number in the directory path here.
| @@ -0,0 +1,67 @@ | |||
| { | |||
| "parameters": { | |||
| "subscriptionId": "e68d4145-c9ae-4667-925d-c51c8d88ad73", | |||
There was a problem hiding this comment.
nit: we recommend not exposing production style subscriptionids
|
@sergey-shandar |
| "swagger": "2.0", | ||
| "info": { | ||
| "title": "TrafficManagerManagementClient", | ||
| "version": "2018-03-01" |
There was a problem hiding this comment.
folder name is 2018-04-01 but the version here is still 2018-03-01. Do you intend to add the APIs in a newer APi-version or existing?
| } | ||
| } | ||
| }, | ||
| "put": { |
There was a problem hiding this comment.
PUT on a collection is not allowed/supported by ARM. Please include the instancename in the URL.
It should be /subscriptions/{subscriptionId}/providers/Microsoft.Network/trafficManagerUserMetricsKeys/{metricKeyName}. If it does not really have a name, you can put a default or current as the segment. The pattern wont work for ARM in the current shape. Check other resource types in this spec for reference.
There was a problem hiding this comment.
Should be /subscriptions/{subscriptionId}/providers/Microsoft.Network/trafficManagerUserMetricsKeys/default
| } | ||
| } | ||
| }, | ||
| "delete": { |
There was a problem hiding this comment.
same comment as above.
| }, | ||
| "UserMetricsProperties": { | ||
| "properties": { | ||
| "key": { |
There was a problem hiding this comment.
what does this key indicate and how is it used? is it considered a secret? Is it readOnly that the service generates for the user or something which needs to be provided by the user.
There was a problem hiding this comment.
It is not a secret. Is it essentially a unique ID that is shared knowledge between the subscription and Microsoft. It is read-only. They "PUT" to create (or refresh) a key, and then we create it for them.
| } | ||
| } | ||
| }, | ||
| "/subscriptions/{subscriptionId}/providers/Microsoft.Network/trafficManagerUserMetricsKeys": { |
There was a problem hiding this comment.
Is it at subscription scope or should it live at resource group scope?
There was a problem hiding this comment.
It is at subscription scope.
|
@tatlicioglu - please take a look at the comments and feel free to set something up if there are questions. |
| - tag: package-2017-05 | ||
| - tag: package-2017-09-preview-only | ||
| - tag: package-2018-02 | ||
| - tag: package-2018-03 |
There was a problem hiding this comment.
@tatlicioglu where are these tags (2018-02, 2018-03) come from? Ruby is failing because of the tags.
There was a problem hiding this comment.
I don't think this is supposed to be part of the PR. Omitted them from the new PR (#3361)
There was a problem hiding this comment.
Okay I figured this out. readme.md was missed from this PR. Adding in 3361.
|
I have created a new PR #3361 since I did not have auth to update Mehmet's PR. The two PR's cross-reference one another now. All above comments have been addressed in 3361. |
|
See #3361 |
Automation for azure-sdk-for-javaA PR has been created for you: |
6 participants
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
api-versionin the path should match theapi-versionin the spec).Quality of Swagger