-
Notifications
You must be signed in to change notification settings - Fork 25k
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
Painless Context Doc: Add filter context example #35305
Painless Context Doc: Add filter context example #35305
Conversation
Pinging @elastic/es-core-infra |
some vagrant tests failed not related to this PR |
This script allows to find all documents where the value of `doc['sold']` | ||
is `false`, and where the cost is less than 18. | ||
|
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.
Instead of "allows to find", I'd just say "finds". Given the straightforwardness of the example, I'd just summarize the intent of the script, instead of repeating what the code says:
This script finds all unsold documents that cost less than $18.
to filter all theatre seats for evening performances that are not | ||
sold yet, and where the cost is less than `params.cost`: | ||
|
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'd tweak this slightly to put the emphasis on parameterizing cost:
Defining cost
as a script parameter enables the cost to be configured in the script query request. For example,
the following request finds all available theatre seats for evening performances that are under $18.
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.
Tech side of this looks good. Thanks for this example!
@debadair Thank you for your feedback. I have modified the doc file according to it. Please let me know if this PR is ready for the approval. |
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.
LGTM
relates to #34829