Adding ZRS support for snapshots, new apiversion 2018-04-01

[carissasun]

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]

Automation for azure-sdk-for-python

A 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:
Azure/azure-sdk-for-python#1930

[jhendrixMSFT]

@olydis Does this constitute a breaking change for C#?

[carissasun]

this change is needed in order to add new create types, in addition I'm changing it in disk.json so the two need to match

[olydis]

@jhendrixMSFT yes (and for other languages)

[AutorestCI]

Automation for azure-sdk-for-go

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

Command: ['/usr/local/bin/autorest', '/tmp/tmp4wohts_9/rest/specification/compute/resource-manager/readme.md', '--go', '--go-sdk-folder=/tmp/tmp4wohts_9/sdk', '--multiapi', '--package-version=latest', '[email protected]/autorest.go@~2.1.87', "--user-agent='Azure-SDK-For-Go/latest services'", '--verbose']
Finished with return code 1
and output:

AutoRest code generation utility [version: 2.0.4255; node: v7.10.1]
(C) 2018 Microsoft Corporation.
https://aka.ms/autorest
   Loading AutoRest core      '/tmp/.autorest/@[email protected]/node_modules/@microsoft.azure/autorest-core/dist' (2.0.4255)
   Loading AutoRest extension '@microsoft.azure/autorest.go' (~2.1.87->2.1.88)
   Loading AutoRest extension '@microsoft.azure/autorest.modeler' (2.3.38->2.3.38)
Processing batch task - {"tag":"package-2017-12"} .
FATAL: System.InvalidOperationException: Swagger document contains two or more x-ms-enum extensions with the same name 'StorageAccountTypes' and different values: Standard_LRS,Premium_LRS vs. Standard_LRS,Premium_LRS
   at AutoRest.Modeler.ObjectBuilder.BuildServiceType(String serviceTypeName, Boolean required) in C:\Users\ci\AppData\Local\Temp\PUBLISH8kiit\38_20171116T010915\autorest.modeler\src\ObjectBuilder.cs:line 147
   at AutoRest.Modeler.SchemaBuilder.ParentBuildServiceType(String serviceTypeName, Boolean required) in C:\Users\ci\AppData\Local\Temp\PUBLISH8kiit\38_20171116T010915\autorest.modeler\src\SchemaBuilder.cs:line 204
   at AutoRest.Modeler.SchemaBuilder.BuildServiceType(String serviceTypeName, Boolean required) in C:\Users\ci\AppData\Local\Temp\PUBLISH8kiit\38_20171116T010915\autorest.modeler\src\SchemaBuilder.cs:line 46
   at AutoRest.Modeler.SchemaBuilder.BuildServiceType(String serviceTypeName, Boolean required) in C:\Users\ci\AppData\Local\Temp\PUBLISH8kiit\38_20171116T010915\autorest.modeler\src\SchemaBuilder.cs:line 131
   at AutoRest.Modeler.SchemaBuilder.BuildServiceType(String serviceTypeName, Boolean required) in C:\Users\ci\AppData\Local\Temp\PUBLISH8kiit\38_20171116T010915\autorest.modeler\src\SchemaBuilder.cs:line 131
   at AutoRest.Modeler.SwaggerModeler.BuildCompositeTypes() in C:\Users\ci\AppData\Local\Temp\PUBLISH8kiit\38_20171116T010915\autorest.modeler\src\SwaggerModeler.cs:line 271
   at AutoRest.Modeler.SwaggerModeler.Build(ServiceDefinition serviceDefinition) in C:\Users\ci\AppData\Local\Temp\PUBLISH8kiit\38_20171116T010915\autorest.modeler\src\SwaggerModeler.cs:line 66
   at AutoRest.Modeler.Program.<ProcessInternal>d__2.MoveNext() in C:\Users\ci\AppData\Local\Temp\PUBLISH8kiit\38_20171116T010915\autorest.modeler\src\Program.cs:line 60
--- 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: go/imodeler1 - FAILED
FATAL: Error: Plugin imodeler1 reported failure.
Process() cancelled due to exception : Plugin imodeler1 reported failure.
Failure during batch task - {"tag":"package-2017-12"} -- Error: Plugin imodeler1 reported failure..
  Error: Plugin imodeler1 reported failure.

[jhendrixMSFT]

These examples are missing and causing validation failures which must be fixed, please see the log for more details.

[carissasun]

@jhendrixMSFT example files have been added

[carissasun]

@jhendrixMSFT ping

[jhendrixMSFT]

@carissasun has ARM reviewed this new API yet?

[carissasun]

we have emailed @ravbhatnagar

[jhendrixMSFT]

@carissasun has this been reviewed by the ARM team yet?

[ravbhatnagar]

Some minor comments. Looks good overall.

[ravbhatnagar]

Add "default" response to capture the error responses for all APIs. The schema should comply with the ARM error contract schema. Example in Batch RP spec

[ravbhatnagar]

Just wanted to confirm that these should not be returned in the response of GET/PUT/PATCH

[carissasun]

they are returned in the responses

[ravbhatnagar]

This is not the final response of the API, is it? This is the response when someone does a GET on the Azure-Async operation header, correct?

[carissasun]

that's correct, the actual response (what you want to get) is in the body

[ravbhatnagar]

It would be great if you can make this enum and define all the supported values.

[carissasun]

this is also in compute.json, we will not change this unless crp changes it

[ravbhatnagar]

@jhendrixMSFT - Looks good for ARM based on answers to the above comments.

[jhendrixMSFT]

@carissasun there are still quite a few examples missing, please see the linter log (search for disk.json) and the modeler log (search for operation groups added in disk.json). When do you plan to add these?

[AutorestCI]

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

[carissasun]

@jhendrixMSFT that build was from 5 days ago, how do I request a new build? There are 15 example files needed for the disk.json and I've uploaded all 15. I also updated the readme.md so it generates disks from the 2018-04-01 version and everything else from the current version

[jhendrixMSFT]

@carissasun there is a new build with your changes to the readme so we should have results soon.

[jhendrixMSFT]

@carissasun latest build is ready, under allowed failures please see the linter and model tasks where PR_ONLY=true

[carissasun]

@jhendrixMSFT it's saying it's missing examples for both disk.json and compute.json... they mostly say "Found errors in validating the response with statusCode "200" " why do we need these? Also this job is asking for " minimum/maximum property set ", what is this?

[jhendrixMSFT]

Take the following error.

message: 'Following response status codes "200" for operation "Disks_CreateOrUpdate" were present in the swagger spec, however they were not present in x-ms-examples. Please provide them.'

This operation defines responses for 200 and 202 status codes however the example only defines a response for 202. The above error message is telling you that you're missing an example response for 200.

[jhendrixMSFT]

I don't understand what the "minimum/maximum" thing is about (I will follow up on that) but for example Disks_Update doesn't have an example so I suspect this is what it's complaining about.

[carissasun]

@jhendrixMSFT these are existing files though, it's complaining about the 200 for both disk.json and compute.json. We don't actually need these examples.... is there any way to suppress them?

[jhendrixMSFT]

You can ignore the issues outside of compute.json as that was broken before your PR. Why don't you need examples for 200?

[carissasun]

@jhendrixMSFT the response for 200 would be exactly the same as 202 though?

[jhendrixMSFT]

I don't know, it's your service. :) But if that's the case then it should be a simple copy/paste to add the missing responses yes?

[carissasun]

I actually have added them yesterday (copy paste) if you check the commits

…

On Mar 7, 2018, at 9:00 AM, Joel Hendrix ***@***.***> wrote: I don't know, it's your service. :) But if that's the case then it should be a simple copy/paste to add the missing responses yes? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[jhendrixMSFT]

In the example I cited earlier, for its example file CreateAnEmptyManagedDisk.json I only see a 202 response.

[carissasun]

@jhendrixMSFT please check again linkhttps://github.com/carissasun/azure-rest-api-specs/blob/master/specification/compute/resource-manager/Microsoft.Compute/stable/2018-04-01/examples/CreateAnEmptyManagedDisk.json and let me know if this works

[carissasun]

@jhendrixMSFT , the build passed please merge this

[jhendrixMSFT]

@carissasun are you planning on adding the missing examples later? If so can you please reference the issue you opened on github to track that in this PR.

[carissasun]

@jhendrixMSFT what missing examples are you talking about? the travis build passed

[jhendrixMSFT]

There are several jobs that currently run as "allowed failures", the linter and model validation are two such jobs that are relevant here. In the allowed failures section the ones you care about are linter and model validation where PR_ONLY=true. Here is the link to the linter job. If you search for disk.json in the log you will see all of the operations that are missing examples (or perhaps partial examples, e.g. result body for only one of two status codes).

[carissasun]

@jhendrixMSFT they're all saying (Please provide x-ms-examples describing minimum/maximum property set) for disks.json. I don't know what that means. Can you please merge

[vikramd-ms]

@jhendrixMSFT, can we please address the linter warnings for existing swagger later and merge the changes needed for Zones at this time?

[jhendrixMSFT]

@vikramd-ms the errors to which I'm referring are in the disk.json swagger introduced in this PR. If there is urgency to get this merged that's fine, just please open an issue in this repo and paste it in this PR so we can track the work.
@carissasun Let's dissect the error message.

{
  "type": "Error",
  "code": "XmsExamplesRequired",
  "message": "Please provide x-ms-examples describing minimum/maximum property set for response/request payloads for operations. Operation: 'Disks_Update'",
  "id": "R2022",
  "validationCategory": "SDKViolation",
  "providerNamespace": "Microsoft.Compute",
  "resourceType": "disks",
  "sources": [
    "file:///home/travis/build/Azure/azure-rest-api-specs/specification/compute/resource-manager/Microsoft.Compute/stable/2018-04-01/disk.json:103:6 ($.paths[\"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/disks/{diskName}\"].patch)"
  ],
  "jsonref": "file:///home/travis/build/Azure/azure-rest-api-specs/specification/compute/resource-manager/Microsoft.Compute/stable/2018-04-01/disk.json:103:6 ($.paths[\"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/disks/{diskName}\"].patch)",
  "json-path": "file:///home/travis/build/Azure/azure-rest-api-specs/specification/compute/resource-manager/Microsoft.Compute/stable/2018-04-01/disk.json:103:6 ($.paths[\"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Compute/disks/{diskName}\"].patch)"
}

The code is XmsExamplesRequired which means examples are missing. In the message you can see that this error is specific to operation Disks_Update. Go look at disk.json being added in this PR and find operation Disks_Update, you can see that there's no x-ms-examples declared for this operation. And please note as stated earlier, your examples need to include sample responses for each response code for a given operation.

[carissasun]

@jhendrixMSFT exactly, this is an error based on pre-existing json so can we ignore it for now.... The only thing I'm changing in 208-04-01 is adding Standard_ZRS to account types and changing it to modelAsString

[jhendrixMSFT]

@carissasun So you're saying that only some of the content in disk.json is new in this PR? Assuming that's the case please be sure to make that clear in future PRs. When introducing a new API version that augments a previous one it's ideal to first copy the existing one (the original disk.json), commit that, then add the new functionality and updated API version in a separate commit; this way it's very clear what's been added and we can focus on just that part.