-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
[DOCS] Add snippet tests to retriever API docs #113289
Conversation
Documentation preview: |
Pinging @elastic/es-docs (Team:Docs) |
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.
Thanks so much for adding tests!!! 🎉 That's great to see.
I've added one comment, then ready to approve.
{"index":{}} | ||
{"region": "Austria", "year": "2020"} | ||
|
||
PUT /my-embeddings |
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.
Nitpick: It might be nicer to have a single index restaurants
with the combined keyword and vector fields. This would make the examples a bit cleaner.
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.
Totally! I'll update :cat_frantically_typing:
---- | ||
GET /restaurants/_search | ||
GET /restaurants,my-embeddings/_search |
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 goes back to my nitpick in tests, I'd like to see us continue to query a single index or alias in this example
@@ -316,7 +367,7 @@ GET movies/_search | |||
} | |||
} | |||
---- | |||
// NOTCONSOLE | |||
// TEST[skip:uses ELSER] |
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.
👍 Nice comment explanations
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.
Thanks for iterating - and adding these tests!!
We can't test the elser + semantic reranking one, so have to omit those.