SQL breaking changes to fix Swagger spec validation errors/warnings by jaredmoo · Pull Request #2927 · Azure/azure-sdk-for-net

jaredmoo · 2017-03-11T22:15:58Z

AutoRest enabled several new validation rules which exposed some naming issues in SQL swagger specs. I also noticed that several classes are no longer supported.

Removed unsupported types (e.g. Schema, Table, Column)
Revised import export operation and class names
Removed unnecessary import export operation results APIs

Corresponding Swagger change: Azure/azure-rest-api-specs#1005

This checklist is used to make sure that common guidelines for a pull request are followed.

I have read the contribution guidelines.
The pull request does not introduce breaking changes.

General Guidelines

Title of the pull request 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 more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

Pull request includes test coverage for the included changes.

SDK Generation Guidelines

If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as a link to the swagger spec, used to generate the code.
The project.json and AssemblyInfo.cs files have been updated with the new version of the SDK.

msftclas · 2017-03-11T22:16:02Z

@jaredmoo,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

cormacpayne · 2017-03-15T22:28:42Z

This PR should be merged at the same time as #2914 and #2925

jaredmoo · 2017-03-17T02:07:22Z

#2914 and #2925 are now combined into this

jaredmoo · 2017-03-17T17:55:54Z

Swagger changes are all merged now.

jaredmoo · 2017-03-17T17:58:11Z

btw project.json isn't updated because 1.2.0-preview hasn't been published yet.

cormacpayne · 2017-03-17T18:01:37Z

@jaredmoo Hey Jared, two quick questions:

Just to be sure, does this spec include all of the changes you originally wanted to make in this PR and the two that you closed?
There are a lot of unnecessary commits, would you mind rebasing to get the number of commits down to 1 or 2?

Also renamed several types and operations for improved clarify and consistency. Additions: - BlobAuditingPolicy APIs (e.g. Databases.CreateOrUpdateBlobAuditingPolicy) - ThreatDetectionPolicy APIs (e.g. Databases.CreateOrUpdateThreatDetectionPolicy) - Databases.ListByServer now supports $expand parameter - Capabilities APIs (e.g. Capabilities.ListByLocation) Types renamed: - ServerFirewallRule -> FirewallRule - DatabaseEditions -> DatabaseEdition - ElasticPoolEditions -> ElasticPoolEdition - ImportRequestParameters -> ImportRequest - ExportRequestParameters -> ExportRequest - ImportExportOperationResponse -> ImportExportResponse - OperationMode -> ImportOperationMode - TransparentDataEncryptionStates -> TransparentDataEncryptionStatus Types removed: - Unused types: UpgradeHint, Schema, Table, Column Operations renamed: - Servers.GetByResourceGroup -> Servers.Get - Servers.CreateOrUpdateFirewallRule -> FirewallRule.CreateOrUpdate, and similar for Get, List, and Delete - Databases.Import -> Databases.CreateImportOperation - Servers.Import -> Databases.Import - Databases.PauseDataWarehouse -> Databases.Pause - Databases.ResumeDataWarehouse -> Databases.Resume Operations removed: - Removed ImportExport operation results APIs since these are handled automatically by Azure async pattern.

jaredmoo · 2017-03-17T19:37:49Z

Yes, since all swagger changes are now merged, all generated files in this change are based on rest-api-specs master branch.

I have squashed into a single commit and described all the breaking changes.

jaredmoo · 2017-03-20T16:14:46Z

@cormacpayne can I get a review?

cormacpayne

@jaredmoo some comments that need to be resolved

Also, what is the UpgradeLog.htm file? I was unable to view it on GitHub or CodeFlow

cormacpayne · 2017-03-20T18:12:39Z

global.json

    "src/ResourceManagement/Monitor/Microsoft.Azure.Monitor"
-  ]
+  ],
+  "sdk": {


@jaredmoo why was this added?

Without this the build is totally broken on my machine, because I have VS2017 installed. See #2922 .

cormacpayne · 2017-03-20T18:12:50Z

src/ClientRuntime/global.json

-                "Microsoft.Rest.ClientRuntime.Log4Net", "Microsoft.Rest.ClientRuntime.Tracing.Tests" ]
+                "Microsoft.Rest.ClientRuntime.Log4Net", "Microsoft.Rest.ClientRuntime.Tracing.Tests" ],
+  "sdk": {
+    "version": "1.0.0-preview2-003121"


@jaredmoo same comment

cormacpayne · 2017-03-20T18:13:10Z

src/ResourceManagement/SqlManagement/global.json

-  "projects": [ "../../TestFramework", "Microsoft.Azure.Management.Sql", "Sql.Tests" ]
+  "projects": [ "../../TestFramework", "Microsoft.Azure.Management.Sql", "Sql.Tests" ],
+  "sdk": {
+    "version": "1.0.0-preview2-003121"


@jaredmoo same comment

cormacpayne · 2017-03-20T18:13:41Z

src/TestFramework/global.json

-  "projects": [ "Microsoft.Rest.ClientRuntime.Azure.TestFramework", "Microsoft.Azure.Test.HttpRecorder" ]
+  "projects": [ "Microsoft.Rest.ClientRuntime.Azure.TestFramework", "Microsoft.Azure.Test.HttpRecorder" ],
+  "sdk": {
+    "version": "1.0.0-preview2-003121"


@jaredmoo same comment

cormacpayne · 2017-03-20T18:29:36Z

src/ResourceManagement/SqlManagement/Sql.Tests/ImportExportScenarioTests.cs

                    });
                }
-
-                // List Server import/export operations


@jaredmoo is this removed block being tested somewhere else? Making sure there isn't any test regression with this being removed

This API is removed so no test is needed.

cormacpayne · 2017-03-20T18:30:32Z

src/ResourceManagement/SqlManagement/Sql.Tests/BlobAuditingTest.cs

@@ -0,0 +1,183 @@
+using Microsoft.Azure.Management.Resources;


@jaredmoo this file is missing the license header

Fixed in 8350101

cormacpayne · 2017-03-20T18:31:09Z

src/ResourceManagement/SqlManagement/Sql.Tests/ThreatDetectionTest.cs

@@ -0,0 +1,166 @@
+using Microsoft.Azure.Management.Resources;


@jaredmoo this file is missing license header

Fixed in 8350101

jaredmoo · 2017-03-20T18:41:40Z

UpgradeLog.htm is visual studio junk. Delete it.

jaredmoo · 2017-03-21T21:40:41Z

@cormacpayne - I have addressed feedback. Not changing project.json since then it doesn't build for me. I think project.json goes away when #2815 is done.

cormacpayne · 2017-03-21T22:58:24Z

@jaredmoo it looks like there is a merge conflict from a pull request that was merged earlier today

jaredmoo · 2017-03-21T23:06:38Z

Ok, I reverted my change to this file in order to resolve the conflict.

jaredmoo · 2017-03-22T02:45:34Z

@cormacpayne ready to merge again :)

msftclas added the cla-not-required label Mar 11, 2017

jaredmoo mentioned this pull request Mar 11, 2017

Removed no longer supported SQL resource types and renamed several operations Azure/azure-rest-api-specs#1005

Merged

10 tasks

jaredmoo added the spec-not-merged label Mar 11, 2017

This was referenced Mar 15, 2017

SQL database auditing and threat detection #2925

Closed

Initial SQL Capabilities SDK commit, with Capabilities API and comments #2914

Closed

jaredmoo removed the spec-not-merged label Mar 17, 2017

jaredmoo force-pushed the breakingchanges branch from cd65b6a to 5ef6c83 Compare March 17, 2017 18:50

cormacpayne added the needs-review label Mar 17, 2017

cormacpayne suggested changes Mar 20, 2017

View reviewed changes

cormacpayne added needs-revision and removed needs-review labels Mar 20, 2017

jaredmoo added 2 commits March 20, 2017 11:37

Added license header

8350101

Removed UpgradeLog.htm

a9e5599

Merge branch 'AutoRest' into breakingchanges

5ba7652

Reverted global.json changes

f6ecaf8

cormacpayne approved these changes Mar 23, 2017

View reviewed changes

cormacpayne merged commit 027356f into Azure:AutoRest Mar 23, 2017

jaredmoo deleted the breakingchanges branch June 7, 2017 03:00

		@@ -0,0 +1,183 @@
		using Microsoft.Azure.Management.Resources;

		@@ -0,0 +1,166 @@
		using Microsoft.Azure.Management.Resources;

Conversation

jaredmoo commented Mar 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

msftclas commented Mar 11, 2017

Uh oh!

cormacpayne commented Mar 15, 2017

Uh oh!

jaredmoo commented Mar 17, 2017

Uh oh!

jaredmoo commented Mar 17, 2017

Uh oh!

jaredmoo commented Mar 17, 2017

Uh oh!

cormacpayne commented Mar 17, 2017

Uh oh!

jaredmoo commented Mar 17, 2017

Uh oh!

jaredmoo commented Mar 20, 2017

Uh oh!

cormacpayne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaredmoo Mar 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jaredmoo commented Mar 20, 2017

Uh oh!

jaredmoo commented Mar 21, 2017

Uh oh!

cormacpayne commented Mar 21, 2017

Uh oh!

jaredmoo commented Mar 21, 2017

Uh oh!

jaredmoo commented Mar 22, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jaredmoo commented Mar 11, 2017 •

edited

Loading

jaredmoo Mar 20, 2017 •

edited

Loading