Swagger changes for adding a new API version in Backup #7483

gaalla · 2019-10-14T15:19:59Z

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

I have reviewed the documentation for the workflow.
Validation tools were run on swagger spec(s) and have all been fixed in this PR.
The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
adding/removing APIs.
adding/removing properties.
adding/removing API-version.
adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
Please follow the link to find more details on API review process.

openapi-sdkautomation · 2019-10-14T15:20:02Z

azure-sdk-for-js - Release

⚠️ warning [Logs] [Expand Details]

⚠️ Generate from abe9eb0 with merge commit d9d6031. SDK Automation 13.0.17.20191226.1
```
Failed to find any diff after autorest so no changed packages was found.
```

msftclas · 2019-10-14T15:20:12Z

All CLA requirements met.

azuresdkci · 2019-10-14T15:20:54Z

Can one of the admins verify this patch?

AutorestCI · 2019-10-14T15:21:31Z

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#8077

AutorestCI · 2019-10-14T15:22:47Z

Automation for azure-sdk-for-go

A PR has been created for you:
Azure/azure-sdk-for-go#6020

sanjaiganesh · 2019-10-16T01:43:52Z

"/Subscriptions/{subscriptionId}/providers/Microsoft.RecoveryServices/locations/{azureRegion}/backupCrossRegionRestore":{

as this is restore action, /restoreCrossRegionBackup makes sense.

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:57 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:44:44Z

},

add default response (error)

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:91 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:45:03Z

    "responses":{

specify how async is tracked (below assumes it is via azure ascyn)

"x-ms-long-running-operation": true,
"x-ms-long-running-operation-options": {
"final-state-via": "azure-async-operation"
}

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:87 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:46:20Z

    "operationId":"CrossRegionRestore_Trigger",

_ is more meaningful

so, how about CrossRegionBackup_Restore

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:63 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:46:58Z

    "operationId":"CrrJobDetails_Get",

update (not get operation).

CrrJob_Backup ?

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:105 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:47:45Z

    "operationId":"CrrJobDetails_Get",

Update Summary as well

In reply to: 542474132 [](ancestors = 542474132)

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:105 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:48:39Z

  "post":{

Is this really POST ? or GET ?

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:100 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:48:59Z

        "$ref":"./examples/AzureIaasVm/GetCrrJob_Post.json"

point to the right json for GET.. assuming thsiis GET

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:130 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:49:13Z

  "post":{

same feedback here.. Is this really POST ? or GET ? ..

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:136 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:49:19Z

        "$ref":"./examples/AzureIaasVm/ListCrrJobs_Post.json"

point to the right example

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:169 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:50:07Z

    "operationId":"BackupCrrJobs_List",

define another api for _ListBySubscription (GET /Subscriptions/{subscriptionId}/providers/Microsoft.RecoveryServices/backupCrrJobs"

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:141 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:50:42Z

},

specify how async is tracked (below assumes it is via azure ascyn)

"x-ms-long-running-operation": true,
"x-ms-long-running-operation-options": {
"final-state-via": "azure-async-operation"
}

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:206 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:50:50Z

    "responses":{

add default response (error)(

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:200 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:51:03Z

      "500":{

no need to specify this.. add default response

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:207 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

sanjaiganesh · 2019-10-16T01:51:31Z

    "responses":{

add default response

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:120 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

AutorestCI · 2020-01-23T06:44:36Z

You don't have permission to trigger SDK Automation.
Please add yourself to Azure group from opensource portal if you are MSFT employee,
or please ask reviewer to add comment *** /openapibot sdkautomation ***.
Please ask [email protected] (or NullMDR in github) for additional help.

gaalla · 2020-01-23T07:14:34Z

    "responses":{
This doesn't seem to be addressed.

In reply to: 542473776 [](ancestors = 542473776)

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:87 in 6dcce8b. [](commit_id = 6dcce8b, deletion_comment = False)

Fixing this.

gaalla · 2020-01-23T07:25:29Z

  "get":{
Making this a GET is problematic. The ARM expectation is that GETs are reserved for requests that behave either as a read-only or CRUD-able resource. This is neither and as such should be a POST.

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:22 in f7cdddb. [](commit_id = f7cdddb, deletion_comment = False)

This API tries to fetch the AAD Properties for a stamp in a particular region. The AADPropertiesResource is a read only resource and hence doing a GET on it. Please let me know if this doesn't look right to you.

gaalla · 2020-01-23T08:19:17Z

"/Subscriptions/{subscriptionId}/providers/Microsoft.RecoveryServices/locations/{azureRegion}/backupCrrJob":{
Using "backup" as a routing prefix results in confusing action names and is basically an anti-pattern. Without knowing anything else an action called backupCrrJob implies that the Crr Job will be backed up, which is not the case.

I understand that the current design has a constraint on the prefix, so I'll let that go, but the action should make it obvious what it does. A name like backupGetCrrJob would work. Modeling it as a proxy resource would be even better.

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:99 in f7cdddb. [](commit_id = f7cdddb, deletion_comment = False)

I completely understand the concern here. The reason for selecting this name is because we have two APIs called backupJobs and backupJob already to fetch jobs. Now while adding these two APIs for cross region restore jobs I used the same naming convention to keep it consistent with the existing APIs. Let me know if this works.

gaalla · 2020-01-23T08:19:47Z

"/Subscriptions/{subscriptionId}/providers/Microsoft.RecoveryServices/locations/{azureRegion}/backupCrrJobs":{
This differs by one letter from the backupCrrJob action. It should be called backupListCrrJobs.

Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:135 in f7cdddb. [](commit_id = f7cdddb, deletion_comment = False)

Same comment as above.

AutorestCI · 2020-01-23T08:20:42Z

You don't have permission to trigger SDK Automation.
Please add yourself to Azure group from opensource portal if you are MSFT employee,
or please ask reviewer to add comment *** /openapibot sdkautomation ***.
Please ask [email protected] (or NullMDR in github) for additional help.

majastrz · 2020-01-23T21:48:30Z

  "get":{
Making this a GET is problematic. The ARM expectation is that GETs are reserved for requests that behave either as a read-only or CRUD-able resource. This is neither and as such should be a POST.
Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:22 in f7cdddb. [](commit_id = f7cdddb, deletion_comment = False)
This API tries to fetch the AAD Properties for a stamp in a particular region. The AADPropertiesResource is a read only resource and hence doing a GET on it. Please let me know if this doesn't look right to you.

The path here is /Subscriptions/{subscriptionId}/providers/Microsoft.RecoveryServices/locations/{azureRegion}/backupAadProperties

The issue is that you are returning the payload containing a single resource on what ARM considers a LIST path. The response from that PATH should be:

{ 
  "value": [ 
    { ... }
  ] 
}

There is a pre-approved singleton resource pattern, which uses a path like this:
/Subscriptions/{subscriptionId}/providers/Microsoft.RecoveryServices/locations/{azureRegion}/backupAadProperties/default

(master or current are also allowed with the singleton pattern instead of default)

AutorestCI · 2020-01-30T07:30:54Z

You don't have permission to trigger SDK Automation.
Please add yourself to Azure group from opensource portal if you are MSFT employee,
or please ask reviewer to add comment *** /openapibot sdkautomation ***.
Please ask [email protected] (or NullMDR in github) for additional help.

gaalla · 2020-01-30T07:32:20Z

  "get":{
Making this a GET is problematic. The ARM expectation is that GETs are reserved for requests that behave either as a read-only or CRUD-able resource. This is neither and as such should be a POST.
Refers to: specification/recoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json:22 in f7cdddb. [](commit_id = f7cdddb, deletion_comment = False)
This API tries to fetch the AAD Properties for a stamp in a particular region. The AADPropertiesResource is a read only resource and hence doing a GET on it. Please let me know if this doesn't look right to you.
The path here is /Subscriptions/{subscriptionId}/providers/Microsoft.RecoveryServices/locations/{azureRegion}/backupAadProperties

The issue is that you are returning the payload containing a single resource on what ARM considers a LIST path. The response from that PATH should be:
{ 
  "value": [ 
    { ... }
  ] 
}
There is a pre-approved singleton resource pattern, which uses a path like this:
/Subscriptions/{subscriptionId}/providers/Microsoft.RecoveryServices/locations/{azureRegion}/backupAadProperties/default

(master or current are also allowed with the singleton pattern instead of default)

Okay. I have added the default keyword in this API. I will take the service side change in the next release.

majastrz

Signed off from ARM side.

...ecoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json

gaalla · 2020-02-10T10:22:15Z

@majastrz / @jhendrixMSFT Can you please merge these changes?

jhendrixMSFT · 2020-02-10T16:02:51Z

/azp run automation - sdk

azure-pipelines · 2020-02-10T16:03:02Z

Azure Pipelines successfully started running 1 pipeline(s).

openapi-sdkautomation · 2020-02-10T16:04:30Z

azure-sdk-for-java - Release

️✔️ succeeded [Logs] [Expand Details]

️✔️ Generate from abe9eb0 with merge commit d9d6031. SDK Automation 13.0.17.20191226.1
️✔️sdk/recoveryservices.backup/mgmt-v2016_08_10 [Logs] [Release SDK Changes]

openapi-sdkautomation · 2020-02-10T16:04:31Z

azure-sdk-for-python - Release

⚠️ warning [Logs] [Expand Details]

⚠️ Generate from abe9eb0 with merge commit d9d6031. SDK Automation 13.0.17.20191226.1
```
Failed to find any diff after autorest so no changed packages was found.
```

openapi-sdkautomation · 2020-02-10T16:04:32Z

azure-sdk-for-net - Release

️✔️ succeeded [Logs] [Expand Details]

️✔️ Generate from abe9eb0 with merge commit d9d6031. SDK Automation 13.0.17.20191226.1

[AutoRest] realpath(): Permission denied
[AutoRest] realpath(): Permission denied
[AutoRest] realpath(): Permission denied
[AutoRest] realpath(): Permission denied
[AutoRest] realpath(): Permission denied
[AutoRest] realpath(): Permission denied

️✔️Microsoft.Azure.Management.RecoveryServices.Backup [Logs] [Release SDK Changes]
- Microsoft.Azure.Management.RecoveryServices.Backup.4.0.1-preview.nupkg
- Microsoft.Azure.Management.RecoveryServices.Backup.4.0.1-preview.snupkg

openapi-sdkautomation · 2020-02-10T16:04:37Z

azure-sdk-for-go - Release

⚠️ warning [Logs] [Expand Details]

⚠️ Generate from abe9eb0 with merge commit d9d6031. SDK Automation 13.0.17.20191226.1
⚠️recoveryservices/mgmt/2016-06-01 [Logs]
```
No file is changed.
```
⚠️recoveryservices/mgmt/2016-12-01 [Logs]
```
No file is changed.
```
️✔️recoveryservices/mgmt/2017-07-01 [Logs] [Release SDK Changes]
- recoveryservices.mgmt.2017-07-01.zip
⚠️recoveryservices/mgmt/2019-05-13 [Logs]
```
No file is changed.
```
⚠️recoveryservices/mgmt/2019-06-15 [Logs]
```
No file is changed.
```

* Swagger changes for adding a new API version in Backup * Adding 2 list APIs specific to the new version * Resolving comments and fixing spell check errors * Fixing test errors * Fixing lint diff errors * Fixing lint diff errors * Fixing AAD properties API * Fixing semantic error and removing HTTP 400 code * Removing error code from example as well * Removing default keyword as it is not supported by our RP * Removing 2019-05-13 version from existing package * Resolving comments * Resolving comments

Swagger changes for adding a new API version in Backup

8eedd70

azuresdkci assigned jhendrixMSFT Oct 14, 2019

azuresdkci requested a review from jhendrixMSFT October 14, 2019 15:20

AutorestCI mentioned this pull request Oct 14, 2019

[AutoPR recoveryservicesbackup/resource-manager] Swagger changes for adding a new API version in Backup Azure/azure-sdk-for-python#7860

Closed

AutorestCI mentioned this pull request Oct 14, 2019

[AutoPR recoveryservicesbackup/resource-manager] Swagger changes for adding a new API version in Backup Azure/azure-sdk-for-go#6020

Closed

jhendrixMSFT added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Oct 14, 2019

Adding 2 list APIs specific to the new version

6dcce8b

sanjaiganesh added the ARMChangesRequested label Oct 16, 2019

Removing 2019-05-13 version from existing package

0173c41

Resolving comments

a7c45c0

Resolving comments

abe9eb0

majastrz added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed ARMChangesRequested labels Jan 31, 2020

majastrz approved these changes Jan 31, 2020

View reviewed changes

jhendrixMSFT reviewed Jan 31, 2020

View reviewed changes

...ecoveryservicesbackup/resource-manager/Microsoft.RecoveryServices/stable/2018-12-20/bms.json Show resolved Hide resolved

jhendrixMSFT approved these changes Feb 10, 2020

View reviewed changes

jhendrixMSFT merged commit d9d6031 into Azure:master Feb 10, 2020

Swagger changes for adding a new API version in Backup #7483

Swagger changes for adding a new API version in Backup #7483

Uh oh!

Conversation

gaalla commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Latest improvements:

Contribution checklist:

ARM API Review Checklist

Uh oh!

openapi-sdkautomation bot commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

azure-sdk-for-js - Release

Uh oh!

msftclas commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

azuresdkci commented Oct 14, 2019

Uh oh!

AutorestCI commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-python

Uh oh!

AutorestCI commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Automation for azure-sdk-for-go

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

sanjaiganesh commented Oct 16, 2019

Uh oh!

AutorestCI commented Jan 23, 2020

Uh oh!

gaalla commented Jan 23, 2020

Uh oh!

gaalla commented Jan 23, 2020

Uh oh!

gaalla commented Jan 23, 2020

Uh oh!

gaalla commented Jan 23, 2020

Uh oh!

AutorestCI commented Jan 23, 2020

Uh oh!

majastrz commented Jan 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AutorestCI commented Jan 30, 2020

Uh oh!

gaalla commented Jan 30, 2020

Uh oh!

majastrz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gaalla commented Oct 14, 2019 •

edited

Loading

openapi-sdkautomation bot commented Oct 14, 2019 •

edited

Loading

msftclas commented Oct 14, 2019 •

edited

Loading

AutorestCI commented Oct 14, 2019 •

edited

Loading

AutorestCI commented Oct 14, 2019 •

edited

Loading

majastrz commented Jan 23, 2020 •

edited

Loading

openapi-sdkautomation bot commented Feb 10, 2020 •

edited

Loading

openapi-sdkautomation bot commented Feb 10, 2020 •

edited

Loading

openapi-sdkautomation bot commented Feb 10, 2020 •

edited

Loading

openapi-sdkautomation bot commented Feb 10, 2020 •

edited

Loading