-
Notifications
You must be signed in to change notification settings - Fork 5.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Azure Search] Swagger specs and examples for data plane GA version 2… #2843
Conversation
…017-11-11 This updates the Swagger specs for the new GA version of the Azure Search data plane. These new specs are exact copies of the 2016-09-01-Preview specs, with the api-version changed. Same for the examples.
Automation for azure-sdk-for-goNothing to generate for azure-sdk-for-go |
Automation for azure-libraries-for-javaNothing to generate for azure-libraries-for-java |
Automation for azure-sdk-for-pythonNothing to generate for azure-sdk-for-python |
Automation for azure-sdk-for-nodeNothing to generate for azure-sdk-for-node |
@jianghaolu -- Can you please look at this today? |
"description": "A value indicating whether the original token will be kept. Default is false." | ||
} | ||
}, | ||
"description": "Converts alphabetic, numeric, and symbolic Unicode characters which are not in the first 127 ASCII characters (the \"Basic Latin\" Unicode block) into their ASCII equivalents, if such equivalents exist. T |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This parameter is required but is missing in the examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't tell from the context -- Which parameter are you referring to?
"x-ms-parameter-location": "client" | ||
}, | ||
"SearchDnsSuffixParameter": { | ||
"name": "searchDnsSuffix", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This required parameter is not provided in any example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do downstream consumers of these examples do anything useful with x-ms-parameterized-host parameters like this one? For example, when we start generating REST API documentation from Swagger (we aren't yet for our data plane, for historical reasons), will the examples in the docs put this parameter and searchServiceName
in the host name part of the request URL where it should be?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doc team is using the x-ms-examples for generating examples in the docs. I think they are doing that. Here is an example for the Storage Management API. Please contact the doc team for feature requests or any feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amarzavery Do you happen to know if these KeyVault docs are auto-generated as well? This seems to be an example of what we're hoping for.
"description": "A value indicating whether the original token will be kept. Default is false." | ||
} | ||
}, | ||
"description": "Converts alphabetic, numeric, and symbolic Unicode characters which are not in the first 127 ASCII characters (the \"Basic Latin\" Unicode block) into their ASCII equivalents, if such equivalents exist. T |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add this required parameter to the examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't tell from the context -- Which parameter are you referring to?
}, | ||
"dataDeletionDetectionPolicy": { | ||
"@odata.type": "#Microsoft.Azure.Search.SoftDeleteColumnDeletionDetectionPolicy", | ||
"softDeleteColumnName": "isDeleted", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These 2 properties are not defined in the type definition of "DataDeletionDetectionPolicy".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a polymorphic type. Note the use of the discriminator
property in DataDeletionDetectionPolicy
. The value of @odata.type
indicates that this object is of type SoftDeleteColumnDeletionPolicy
. If you look at the definition of SoftDeleteColumnDeletionPolicy
in the Swagger, you'll see the softDeleteColumnName
and softDeleteMarkerValue
properties are defined. Also note the use of x-ms-discriminator-value
and allOf
in SoftDeleteColumnDeletionPolicy
.
Please ensure your validation tools understand polymorphism, as it is a crucial feature for modeling data plane APIs using AutoRest.
"description": "another cool indexer", | ||
"dataSourceName": "myblobdatasource", | ||
"targetIndexName": "orders", | ||
"schedule": { }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"interval" is required here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the example is meant to show that the schedule itself is not required. I'll remove it.
The GitHub review link is broken - The major valid error the tool is reporting is missing required parameters from the global "parameters" section, like "Prefer", or missing required parameters on the parameterized host, like "searchDnsSuffix". |
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: File: AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback Thanks for your co-operation. |
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: File: AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback Thanks for your co-operation. |
…017-11-11
This updates the Swagger specs for the new GA version of the Azure Search data
plane. These new specs are exact copies of the 2016-09-01-Preview specs, with
the api-version changed. Same for the examples.
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
api-version
in the path should match theapi-version
in the spec).Quality of Swagger