[Azure Search] Resolve Swagger linter errors for management API #2242

brjohnstmsft · 2018-01-10T00:13:40Z

I recommend reviewing each commit separately.

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.

Support for PATCH was added to the Azure Search RP. This change adds it to the Swagger spec so it will be documented and included in generated SDK code.

…rrors

It doesn't seem to be supported by the .NET ClientRuntime for resource-based polling.

brjohnstmsft · 2018-01-10T00:26:26Z

FYI @Yahnoosh @natinimni @mhko @chaosrealm

Yahnoosh · 2018-01-10T02:00:21Z

...earch/resource-manager/Microsoft.Search/stable/2015-08-19/examples/SearchDeleteQueryKey.json

+    "parameters": {
+        "searchServiceName": "mysearchservice",
+        "resourceGroupName": "rg1",
+        "key": "Query key for browser-based clients",


Query key for browser-based clients [](start = 16, length = 35)

since this is an examples, should we put a fake key here?

You found a bug in the example. Query keys are supposed to be deleted by value, not by name.

I'll use the same placeholder syntax here that I've used elsewhere. Looking at examples in other Swagger specs, I don't see any of the other Azure services using fake values. Some reasons I can think of for this include:

Avoiding accidentally putting a real value in the examples.

Avoiding triggering cred-scanning false positives.

Make sense. The two points is what I though you were trying to avoid by putting the description of what the value is supposed to be, using a placeholder is a better idea.

Yahnoosh

azuresdkciprbot · 2018-01-10T02:43:05Z

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/search/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 12
After the PR: Warning(s): 0 Error(s): 0

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

jianghaolu · 2018-01-10T19:22:02Z

specification/search/resource-manager/Microsoft.Search/stable/2015-08-19/search.json

@@ -969,17 +906,15 @@
          "description": "Tags to help categorize the resource in the Azure portal."
        }
      },
-      "required": [


why is "location" not required anymore?

Same as above

jianghaolu · 2018-01-10T19:24:03Z

specification/search/resource-manager/Microsoft.Search/stable/2015-08-19/search.json

@@ -819,29 +773,12 @@
          }
        }
      },
-      "required": [


I think I understand the reason to remove them as required - I could be wrong, but are you trying to allow "SearchService" to be an acceptable request payload for the "patch" operation? In that case, it might help to create some type like SearchServicePatchParameters?

Right, I want the client programming model to be simpler by using the same type for both operations.

But that way "location" and "sku" are not validated in the create scenarios. @veronicagg Do you have any guidelines which is the recommended way?

They are still validated by the RP. This is just a tradeoff between stronger type safety on the client versus fewer classes for the customer to deal with.

Discussed with Veronica offline - @brjohnstmsft Do you think you can add some descriptions to "sku" property so that users will know this is required in a create scenario?

Sure, I'll make that change.

…on Create

azuresdkciprbot · 2018-01-10T22:48:00Z

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/search/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 12
After the PR: Warning(s): 0 Error(s): 0

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

AutorestCI · 2018-01-10T23:07:54Z

Azure/azure-sdk-for-ruby#1230

AutorestCI · 2018-01-10T23:08:10Z

Azure/azure-sdk-for-python#1766

brjohnstmsft added 6 commits January 9, 2018 13:42

[Azure Search] Adding PATCH Services to Swagger

8a507a0

Support for PATCH was added to the Azure Search RP. This change adds it to the Swagger spec so it will be documented and included in generated SDK code.

[Azure Search] Adding x-ms-examples to Management plane Swagger spec

6e4a646

[Azure Search] Removing required properties to resolve PATCH linter e…

6cad5e0

…rrors

[Azure Search] Removing x-ms-long-running-operation from PATCH

b4ebe23

It doesn't seem to be supported by the .NET ClientRuntime for resource-based polling.

[Azure Search] Fixing a few Swagger linter warnings

84385fa

[Azure Search] Adding Operations API to management plane Swagger

6a31c5b

AutorestCI mentioned this pull request Jan 10, 2018

Automatic PR of restapi_auto_brjohnstmsft_update-search-mgmt-spec into master Azure/azure-sdk-for-python#1757

Merged

azuresdkci assigned jianghaolu Jan 10, 2018

[Azure Search] Fixing validation error in Management API example

57f803b

Yahnoosh reviewed Jan 10, 2018

View reviewed changes

Yahnoosh approved these changes Jan 10, 2018

View reviewed changes

[Azure Search] Fixing broken delete query key example

9e758b9

jianghaolu reviewed Jan 10, 2018

View reviewed changes

shawns1 added the Reassign label Jan 10, 2018

azuresdkci removed the Reassign label Jan 10, 2018

azuresdkci assigned jhendrixMSFT and unassigned jianghaolu Jan 10, 2018

[Azure Search] Clarifying in spec that location and sku are required …

5a97895

…on Create

shawns1 assigned jianghaolu and unassigned jhendrixMSFT Jan 10, 2018

jianghaolu approved these changes Jan 10, 2018

View reviewed changes

jianghaolu merged commit 894d0d8 into Azure:master Jan 10, 2018

brjohnstmsft deleted the update-search-mgmt-spec branch January 10, 2018 23:04

AutorestCI mentioned this pull request Jan 10, 2018

Automatic PR from RestAPI-PR2242 Azure/azure-sdk-for-ruby#1230

Closed

AutorestCI mentioned this pull request Jan 10, 2018

Automatic PR from RestAPI-PR2242 Azure/azure-sdk-for-python#1766

Closed

veronicagg mentioned this pull request Jan 11, 2018

Recommendation for PUT/PATCH model definition in swagger? #2252

Closed

brjohnstmsft mentioned this pull request Jan 11, 2018

[Azure Search] Upgrade Management SDK to version 2.0 Azure/azure-sdk-for-net#3993

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Azure Search] Resolve Swagger linter errors for management API #2242

[Azure Search] Resolve Swagger linter errors for management API #2242

brjohnstmsft commented Jan 10, 2018 •

edited by azuresdkci

Loading

brjohnstmsft commented Jan 10, 2018

Yahnoosh Jan 10, 2018

brjohnstmsft Jan 10, 2018

Yahnoosh Jan 10, 2018

Yahnoosh left a comment

azuresdkciprbot commented Jan 10, 2018

jianghaolu Jan 10, 2018

brjohnstmsft Jan 10, 2018

jianghaolu Jan 10, 2018

brjohnstmsft Jan 10, 2018

jianghaolu Jan 10, 2018

brjohnstmsft Jan 10, 2018

jianghaolu Jan 10, 2018

brjohnstmsft Jan 10, 2018

azuresdkciprbot commented Jan 10, 2018

AutorestCI commented Jan 10, 2018

AutorestCI commented Jan 10, 2018

[Azure Search] Resolve Swagger linter errors for management API #2242

[Azure Search] Resolve Swagger linter errors for management API #2242

Conversation

brjohnstmsft commented Jan 10, 2018 • edited by azuresdkci Loading

PR information

Quality of Swagger

brjohnstmsft commented Jan 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yahnoosh left a comment

Choose a reason for hiding this comment

azuresdkciprbot commented Jan 10, 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

azuresdkciprbot commented Jan 10, 2018

AutorestCI commented Jan 10, 2018

AutorestCI commented Jan 10, 2018

brjohnstmsft commented Jan 10, 2018 •

edited by azuresdkci

Loading