Added BackupValidateOperation API #3395

sumitmal · 2018-07-11T09:59:30Z

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 · 2018-07-11T09:59:57Z

Automation for azure-sdk-for-python

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

AutorestCI · 2018-07-11T10:00:38Z

Automation for azure-sdk-for-go

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

AutorestCI · 2018-07-11T10:01:26Z

Automation for azure-sdk-for-node

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

AutorestCI · 2018-07-11T10:02:05Z

Automation for azure-sdk-for-ruby

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

AutorestCI · 2018-07-11T10:02:06Z

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

jhendrixMSFT

Semantic validation is failing, can you please fix? https://travis-ci.org/Azure/azure-rest-api-specs/jobs/402622342

sumitmal · 2018-07-11T23:23:10Z

@jhendrixMSFT the validation failed with below error, but the file is added at the right location in the PR. Can you let me know what am I missing?

Uncaught Error: /home/travis/build/Azure/azure-rest-api-specs/specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2017-07-01/bms.json has references that cannot be resolved. They are as follows:

'Error opening file "/home/travis/build/Azure/azure-rest-api-specs/specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2017-07-01/examples/AzureIaasVM/ValidateOperation_RestoreDisk.json" \nENOENT: no such file or directory, open '/home/travis/build/Azure/azure-rest-api-specs/specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2017-07-01/examples/AzureIaasVM/ValidateOperation_RestoreDisk.json''
at /home/travis/build/Azure/azure-rest-api-specs/test/syntax.js:72:17
at /home/travis/build/Azure/azure-rest-api-specs/node_modules/call-me-maybe/index.js:13:28
at _combinedTickCallback (internal/process/next_tick.js:131:7)
at process._tickCallback (internal/process/next_tick.js:180:9)

jhendrixMSFT · 2018-07-11T23:27:20Z

CI runs on Linux which has case-sensitive file paths. In part of the path you've specified AzureIaasVM but the file is checked in under AzureIaasVm.

jhendrixMSFT · 2018-07-11T23:38:06Z

...ecoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2016-12-01/bms.json

@@ -4110,12 +4110,7 @@
            "VMAppContainer",
            "SQLAGWorkLoadContainer",
            "StorageContainer",
-            "GenericContainer",


Why are these being removed from a stable spec? This is a breaking change from an SDK perspective. Same comment applies to other places where enums are removed.

GenericContainer should not have been there in first place. The API set is not exposed yet. I will check with the team on why this was there earlier.

When a swagger is merged into the public repo it's considered a release event. As such we've already released this for the Go programming language and possibly others. While we won't prevent you from making this change it will be flagged as a breaking change and reported in S360.

Do you want to leave these breaking changes as-is?

Yes. We discussed this internally. These were not used anywhere. So we will leave them as is.

jhendrixMSFT · 2018-07-11T23:44:12Z

...ecoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2017-07-01/bms.json

+          "Validate"
+        ],
+        "description": "Validate operation for specified backed up item. This is a synchronous operation.",
+        "operationId": "Validate_Operation",


I think we need a better name for this, what this will generate now is a client named Validate with a method named Operation which isn't very intuitive. If this API is to validate an existing backup I would suggest something like Backup_Validate.

Ok. The idea here is to validate the feasibility of the operation. Validate Operation can work before triggering the actual operation.
In example, we try to validate whether the restore operation can happen. API is written generic for all operations.
How about Operation_Validate ?

Ah I see, yeah let's go with Operation_Validate.

jhendrixMSFT · 2018-07-11T23:47:20Z

There are model validation failures for the added API, see the log here, can you please take a look? You can find instructions on running the model validation locally here, the command will be oav validate-example <path to spec>. In the output search for the operation ID "Validate_Operation".

…edback

jhendrixMSFT · 2018-07-13T16:11:16Z

Latest model validation failures log https://travis-ci.org/Azure/azure-rest-api-specs/jobs/403424152

sumitmal · 2018-07-15T23:48:11Z

Not able to understand the Model failure logs? The additional paramaeters are actually part of IaasVMRestoreRequest. I thought if we add objectType (swagger discriminator to the sample, that would fix this.

jhendrixMSFT · 2018-07-16T16:41:23Z

...t.RecoveryServices/stable/2017-07-01/examples/AzureIaasVm/ValidateOperation_RestoreDisk.json

+        "region": "southeastasia",
+        "createNewCloudService": true,
+        "originalStorageAccountOption": false,
+        "api-version": "2016-12-01",


OK regarding the model validation failures, I dug into this some more and the only problem is this extra "api-version" parameter in the request body. The reporting of the other properties appears to be a bug in the validation tool and we are investigating futher.

sumitmal added 2 commits July 11, 2018 15:16

Added BackupValidateOperation API

7e30995

Added BackupValidateOperation API

db66b3c

azuresdkci assigned jhendrixMSFT Jul 11, 2018

azuresdkci requested a review from jhendrixMSFT July 11, 2018 10:00

sumitmal added 2 commits July 11, 2018 17:10

Fix casing in validateOperationResponse

0c40872

Fix casing in validateOperationResponse

53e742c

jhendrixMSFT requested changes Jul 11, 2018

View reviewed changes

Fix casing in swagger example file path

4aaba64

jhendrixMSFT reviewed Jul 11, 2018

View reviewed changes

sumitmal added 3 commits July 13, 2018 09:50

Use ValidateRequest instead of Resource<ValidateRequest>

8d8d07e

Update operationid of validate_Operation to Operation_Validate per fe…

a654352

…edback

Update operationid of validate_Operation to Operation_Validate per fe…

59d0f35

…edback

jhendrixMSFT reviewed Jul 16, 2018

View reviewed changes

remove additional parameter api-version

fd16916

jhendrixMSFT added the potential-sdk-breaking-change label Jul 17, 2018

jhendrixMSFT approved these changes Jul 18, 2018

View reviewed changes

jhendrixMSFT merged commit ccefbaf into Azure:master Jul 18, 2018

This was referenced Jul 27, 2018

RecoveryServices.Backup update for latest release Azure/azure-sdk-for-net#4593

Closed

[Do Not Merge] RecoveryServices.Backup update for latest release Azure/azure-sdk-for-net#4613

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added BackupValidateOperation API #3395

Added BackupValidateOperation API #3395

sumitmal commented Jul 11, 2018 •

edited by azuresdkci

Loading

AutorestCI commented Jul 11, 2018 •

edited

Loading

AutorestCI commented Jul 11, 2018 •

edited

Loading

AutorestCI commented Jul 11, 2018 •

edited

Loading

AutorestCI commented Jul 11, 2018 •

edited

Loading

AutorestCI commented Jul 11, 2018 •

edited

Loading

jhendrixMSFT left a comment

sumitmal commented Jul 11, 2018

jhendrixMSFT commented Jul 11, 2018

jhendrixMSFT Jul 11, 2018

sumitmal Jul 12, 2018

jhendrixMSFT Jul 12, 2018

jhendrixMSFT Jul 17, 2018

sumitmal Jul 18, 2018

jhendrixMSFT Jul 11, 2018

sumitmal Jul 12, 2018

jhendrixMSFT Jul 12, 2018

jhendrixMSFT commented Jul 11, 2018 •

edited

Loading

jhendrixMSFT commented Jul 13, 2018

sumitmal commented Jul 15, 2018

jhendrixMSFT Jul 16, 2018

Added BackupValidateOperation API #3395

Added BackupValidateOperation API #3395

Conversation

sumitmal commented Jul 11, 2018 • edited by azuresdkci Loading

PR information

Quality of Swagger

AutorestCI commented Jul 11, 2018 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Jul 11, 2018 • edited Loading

Automation for azure-sdk-for-go

AutorestCI commented Jul 11, 2018 • edited Loading

Automation for azure-sdk-for-node

AutorestCI commented Jul 11, 2018 • edited Loading

Automation for azure-sdk-for-ruby

AutorestCI commented Jul 11, 2018 • edited Loading

Automation for azure-sdk-for-java

jhendrixMSFT left a comment

Choose a reason for hiding this comment

sumitmal commented Jul 11, 2018

jhendrixMSFT commented Jul 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jhendrixMSFT commented Jul 11, 2018 • edited Loading

jhendrixMSFT commented Jul 13, 2018

sumitmal commented Jul 15, 2018

Choose a reason for hiding this comment

sumitmal commented Jul 11, 2018 •

edited by azuresdkci

Loading

AutorestCI commented Jul 11, 2018 •

edited

Loading

AutorestCI commented Jul 11, 2018 •

edited

Loading

AutorestCI commented Jul 11, 2018 •

edited

Loading

AutorestCI commented Jul 11, 2018 •

edited

Loading

AutorestCI commented Jul 11, 2018 •

edited

Loading

jhendrixMSFT commented Jul 11, 2018 •

edited

Loading