Skip to content
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] Add Get Documents API (a.k.a. Lookup) to data plane API spec #5169

Merged
merged 3 commits into from
Feb 8, 2019

Conversation

brjohnstmsft
Copy link
Member

@brjohnstmsft brjohnstmsft commented Feb 7, 2019

If you are a MSFT employee you can view your work branch via this link.

Contribution checklist:

Note that this API already exists in the .NET SDK, but is currently
implemented as custom code. We need to add transform directives for the
generated code because otherwise there's no way to pass custom
JsonSerializerSettings into the generated code (which is a requirement for
our data plane).

Also renamed some example files to be more consistent with existing naming
conventions.
@AutorestCI
Copy link

AutorestCI commented Feb 7, 2019

Automation for azure-sdk-for-js

Nothing to generate for azure-sdk-for-js

@AutorestCI
Copy link

AutorestCI commented Feb 7, 2019

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

@AutorestCI
Copy link

AutorestCI commented Feb 7, 2019

Automation for azure-sdk-for-python

Nothing to generate for azure-sdk-for-python

@AutorestCI
Copy link

AutorestCI commented Feb 7, 2019

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI
Copy link

AutorestCI commented Feb 7, 2019

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

@brjohnstmsft brjohnstmsft changed the title [Hub Generated] Review request for Microsoft.Azure.Search.Data to add version 2017-11-11-preview [Azure Search] Add Get Documents API (a.k.a. Lookup) to data plane API spec Feb 7, 2019
@AutorestCI
Copy link

AutorestCI commented Feb 7, 2019

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@brjohnstmsft
Copy link
Member Author

Big thanks to @mhko who did the majority of the work on this.

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@brjohnstmsft
Copy link
Member Author

@jhendrixMSFT Some context on this change: This is not a new API, it's just that until now we've never generated code for it. This is the first of many steps we're taking to make this particular Swagger spec accurately reflect our data plane API.

@jhendrixMSFT
Copy link
Member

@brjohnstmsft do you do any local C# codegen to verify this looks ok?

@@ -11,7 +11,7 @@
"highlightPostTag": "</em>",
"highlightPreTag": "<em>",
"minimumCoverage": 80,
"searchFields": ["title", "description"],
"searchFields": "title,description",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec still has it as an array of string

"name":"searchFields",
"in": "query",
"type": "array",
"items": {
"type": "string"
},

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nschonni That's the spec for the GET version of Autocomplete. The example I updated was for POST. AutoRest uses array parameters to represent query string parameters that are comma-separated lists. So although it's a comma-separated list everywhere on the wire, in the spec it's a string for POST and an array of strings for GET.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, I guess it's the alias'd property here

"properties": {
"search": {
"type": "string",
"description": "The search text on which to base autocomplete results.",
"x-ms-client-name": "searchText"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brjohnstmsft
Copy link
Member Author

@jhendrixMSFT Yes, I'm working on a .NET SDK PR that I will send out once this is merged. I wrote the translation rules in tandem with changing the .NET SDK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants