Add Swagger for Short Term Retention Policies #2860

adeal · 2018-04-12T18:33:00Z

This change adds the swagger documentation for the Short Term Retention feature along with examples for each of the APIs. This includes the following APIs:

Get database short term retention policy
Update database short term retention policy

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.

msftclas · 2018-04-12T18:33:11Z

All CLA requirements met.

AutorestCI · 2018-04-12T18:35:20Z

Automation for azure-sdk-for-python

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

AutorestCI · 2018-04-12T18:38:01Z

Automation for azure-libraries-for-java

The initial PR has been merged into your service PR:
AutorestCI/azure-libraries-for-java#9

AutorestCI · 2018-04-12T18:39:42Z

Automation for azure-sdk-for-go

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

AutorestCI · 2018-04-12T18:40:37Z

Automation for azure-sdk-for-node

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

anuchandy · 2018-04-13T00:11:46Z

...ql/resource-manager/Microsoft.Sql/preview/2017-10-01-preview/shortTermRetentionPolicies.json

+              "name": "ShortTermRetentionPolicyName",
+              "modelAsString": true
+            }
+          },


Seems policyName is used in most of the operations, should we move this to parameter section?

Could you expand on what the benefit to that would be?

policyName will always be "default". I'd like to keep it consistent with what we see in Long Term Retention

The benefit would be the same that we get from having a common definition of resource group name or subscription parameter. Out of 8 operations in this swagger, 6 of them uses policyName param, if you centralize this definition then swagger looks more clean, less lines and is a good pattern to follow in general.

In Long Term Rentetion swagger, out of 8 only two uses policyName, so when reviewed it I didn't ask to move definition as it was not saving much.

had an offline chat with @dealaus, swagger is not manually authored, SQL team will discuss internally to see whether this can be accommodated in the swagger gen tool. For now this is not a must fix for this PR.

anuchandy · 2018-04-13T00:20:37Z

@dealaus - you added Don't merge to title - (1) is it because you are still working on this PR? or
(2) because the service is not deployed completely? or some other reason?

If (1) is the case then please let me know once it's ready for review, since this is a new swagger we need to get this reviewed from ARM.

If (2) is the case then it will be helpful to have a approximate date when it is ready for consumption.

adeal · 2018-04-13T01:05:16Z

@anuchandy it is (2) in this case.

Enabling the API is in deployment pipeline right now and has hit some regions (estimated to be WW on 4/20), but the ARM manifest has not been updated with these new APIs.

anuchandy · 2018-04-13T03:11:15Z

@dealaus thanks for providing date. Given swagger is ready, i will request ARM review.

anuchandy · 2018-04-13T03:12:45Z

@ravbhatnagar new swagger, please review

anuchandy · 2018-04-16T18:58:42Z

@jaredmoo it will be very helpful in case you can have a quick look.

nathannfan · 2018-04-16T20:39:00Z

...ql/resource-manager/Microsoft.Sql/preview/2017-10-01-preview/shortTermRetentionPolicies.json

+            "$ref": "#/parameters/ServerNameParameter"
+          },
+          {
+            "name": "restorableDroppedDatabaseId",


Is this a GUID or a name? Also could use a description.

nathannfan · 2018-04-16T20:44:21Z

...ql/resource-manager/Microsoft.Sql/preview/2017-10-01-preview/shortTermRetentionPolicies.json

+          "RestorableDroppedDatabaseShortTermRetentionPolicies"
+        ],
+        "description": "Gets a dropped database's short term retention policy.",
+        "operationId": "RestorableDroppedDatabaseShortTermRetentionPolicies_Get",


RestorableDroppedDatabaseShortTermRetentionPolicies_Get means the operation in .NET will be client.RestorableDroppedDatabaseShortTermRetentionPolicies.CreateOrUpdate()... is there any way you can see to shorten that?

nathannfan · 2018-04-16T20:44:47Z

...ql/resource-manager/Microsoft.Sql/preview/2017-10-01-preview/shortTermRetentionPolicies.json

+          "RestorableDroppedDatabaseShortTermRetentionPolicies"
+        ],
+        "description": "Sets a database's long term retention policy.",
+        "operationId": "RestorableDroppedDatabaseShortTermRetentionPolicies_CreateOrUpdate",


Same comments as Get

nathannfan · 2018-04-16T20:46:52Z

...ql/resource-manager/Microsoft.Sql/preview/2017-10-01-preview/shortTermRetentionPolicies.json

+          "RestorableDroppedDatabaseShortTermRetentionPolicies"
+        ],
+        "description": "Sets a database's long term retention policy.",
+        "operationId": "RestorableDroppedDatabaseShortTermRetentionPolicies_Update",


Same comment as Get

ravbhatnagar

Just one comment. Please fix. Good to merge after this one fix.

ravbhatnagar · 2018-04-18T15:05:21Z

...ql/resource-manager/Microsoft.Sql/preview/2017-10-01-preview/shortTermRetentionPolicies.json

+          "200": {
+            "description": "Successfully retrieved the policy.",
+            "schema": {
+              "$ref": "#/definitions/ShortTermRetentionPolicy"


This is a list API and the response should be a "value" array. Check other API specs for example.

Ok, I will remove from this checkin for now until we can fix the API to return an array. Shouldn't be a problem, it will just take ~14 days to be deployed.

ravbhatnagar · 2018-04-18T15:08:01Z

Just one issue needs to be fixed. Conditionally signed off.

…o return array

anuchandy · 2018-04-20T02:01:50Z

@dealaus can you fix the readme.md conflict?

adeal · 2018-04-20T06:48:02Z

@anuchandy done.

azuresdkciprbot · 2018-04-20T06:53:18Z

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/sql/resource-manager/readme.md
Before the PR: Warning(s): 26 Error(s): 0
After the PR: Warning(s): 26 Error(s): 0

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

azuresdkciprbot · 2018-04-20T07:16:02Z

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/sql/resource-manager/readme.md
Before the PR: Warning(s): 26 Error(s): 0
After the PR: Warning(s): 26 Error(s): 0

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

cleanup old pipelines for RPSaaSMaster

Add short term retention policies specifications

cd82e83

azuresdkci assigned anuchandy Apr 12, 2018

azuresdkci requested a review from anuchandy April 12, 2018 18:34

anuchandy reviewed Apr 13, 2018

View reviewed changes

anuchandy requested a review from ravbhatnagar April 13, 2018 03:12

anuchandy added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Apr 13, 2018

Edit readme to add v4 and include new swagger

bf8a37f

AutorestCI mentioned this pull request Apr 16, 2018

[AutoPR sql/resource-manager] [DO NOT MERGE] Add Swagger for Short Term Retention Policies Azure/azure-sdk-for-go#1605

Closed

anuchandy requested a review from jaredmoo April 16, 2018 18:57

jaredmoo approved these changes Apr 16, 2018

View reviewed changes

nathannfan approved these changes Apr 16, 2018

View reviewed changes

Removing restorable dropped database swagger

e76a513

adeal changed the title ~~[DO NOT MERGE] Add Swagger for Short Term Retention Policies~~ Add Swagger for Short Term Retention Policies Apr 16, 2018

AutorestCI mentioned this pull request Apr 16, 2018

[AutoPR sql/resource-manager] Add Swagger for Short Term Retention Policies Azure/azure-sdk-for-go#1618

Closed

remove restorable dropped database examples

4db4048

AutorestCI mentioned this pull request Apr 16, 2018

[AutoPR sql/resource-manager] Add Swagger for Short Term Retention Policies Azure/azure-sdk-for-go#1622

Closed

ravbhatnagar reviewed Apr 18, 2018

View reviewed changes

ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Apr 18, 2018

Removed ShortTermRetentionPolicies_ListByDatabase until it is fixed t…

15e0663

…o return array

Merge branch 'master' into master

c5b0234

ravbhatnagar closed this Apr 20, 2018

ravbhatnagar reopened this Apr 20, 2018

AutorestCI mentioned this pull request Apr 20, 2018

[AutoPR sql/resource-manager] Add Swagger for Short Term Retention Policies Azure/azure-sdk-for-python#2439

Merged

anuchandy approved these changes Apr 20, 2018

View reviewed changes

anuchandy merged commit 759bd28 into Azure:master Apr 20, 2018

jaredmoo mentioned this pull request May 22, 2018

Add SQL ShortTermRetention, VulnerabilityAssessment, and Elastic jobs Azure/azure-sdk-for-net#4351

Merged

9 tasks

mccleanp pushed a commit that referenced this pull request Mar 23, 2022

Merge pull request #2860 from zhenglaizhang/cleanup-old-pipelines

a413a79

cleanup old pipelines for RPSaaSMaster

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Swagger for Short Term Retention Policies #2860

Add Swagger for Short Term Retention Policies #2860

adeal commented Apr 12, 2018 •

edited

Loading

msftclas commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

anuchandy Apr 13, 2018

adeal Apr 13, 2018 •

edited

Loading

anuchandy Apr 13, 2018

anuchandy Apr 16, 2018

anuchandy commented Apr 13, 2018

adeal commented Apr 13, 2018 •

edited

Loading

anuchandy commented Apr 13, 2018

anuchandy commented Apr 13, 2018

anuchandy commented Apr 16, 2018

nathannfan Apr 16, 2018

nathannfan Apr 16, 2018

nathannfan Apr 16, 2018

nathannfan Apr 16, 2018

ravbhatnagar left a comment

ravbhatnagar Apr 18, 2018

adeal Apr 20, 2018

ravbhatnagar commented Apr 18, 2018

anuchandy commented Apr 20, 2018

adeal commented Apr 20, 2018

azuresdkciprbot commented Apr 20, 2018

azuresdkciprbot commented Apr 20, 2018

Add Swagger for Short Term Retention Policies #2860

Add Swagger for Short Term Retention Policies #2860

Conversation

adeal commented Apr 12, 2018 • edited Loading

PR information

Quality of Swagger

msftclas commented Apr 12, 2018 • edited Loading

AutorestCI commented Apr 12, 2018 • edited Loading

Automation for azure-sdk-for-python

AutorestCI commented Apr 12, 2018 • edited Loading

Automation for azure-libraries-for-java

AutorestCI commented Apr 12, 2018 • edited Loading

Automation for azure-sdk-for-go

AutorestCI commented Apr 12, 2018 • edited Loading

Automation for azure-sdk-for-node

Choose a reason for hiding this comment

adeal Apr 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anuchandy commented Apr 13, 2018

adeal commented Apr 13, 2018 • edited Loading

anuchandy commented Apr 13, 2018

anuchandy commented Apr 13, 2018

anuchandy commented Apr 16, 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

ravbhatnagar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ravbhatnagar commented Apr 18, 2018

anuchandy commented Apr 20, 2018

adeal commented Apr 20, 2018

azuresdkciprbot commented Apr 20, 2018

azuresdkciprbot commented Apr 20, 2018

adeal commented Apr 12, 2018 •

edited

Loading

msftclas commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

AutorestCI commented Apr 12, 2018 •

edited

Loading

adeal Apr 13, 2018 •

edited

Loading

adeal commented Apr 13, 2018 •

edited

Loading