Skip to content

Conversation

@martijnvg
Copy link
Member

Prior to this commit min_score was sent as request body parameter
(via SearchSourceBuilder), which is not possible in the count api.

Similar to #46474

Prior to this commit min_score was sent as request body parameter
(via SearchSourceBuilder), which is not possible in the count api.

Similar to elastic#46474
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

LGTM, I also wonder if we should replace the SearchSourceBuilder with a simple QueryBuilder in order to ensure that we don't send invalid content ?

@martijnvg
Copy link
Member Author

I also wonder if we should replace the SearchSourceBuilder with a simple QueryBuilder in order to ensure that we don't send invalid content ?

I was thinking the same. I will do that in a followup PR.

@martijnvg
Copy link
Member Author

@elasticmachine run elasticsearch-ci/2

@martijnvg martijnvg merged commit 546ded3 into elastic:master Sep 20, 2019
martijnvg added a commit that referenced this pull request Sep 20, 2019
)

Prior to this commit min_score was sent as request body parameter
(via SearchSourceBuilder), which is not possible in the count api.

Similar to #46474
martijnvg added a commit that referenced this pull request Sep 20, 2019
)

Prior to this commit min_score was sent as request body parameter
(via SearchSourceBuilder), which is not possible in the count api.

Similar to #46474
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request Sep 20, 2019
Currently the CountRequest accepts a search source builder, while the
RestCountAction only accepts a top level query object. This can lead
to confusion if another element (e.g. aggregations) is specified,
because that will be ignored on the server side in RestCountAction.

By deprecating the current setter & constructor that accept a
SearchSourceBuilder and adding replacement that accepts a QueryBuilder
it is clear what the count api can handle from HLRC side.

Follow up from elastic#46829
martijnvg added a commit that referenced this pull request Sep 23, 2019
Currently the CountRequest accepts a search source builder, while the
RestCountAction only accepts a top level query object. This can lead
to confusion if another element (e.g. aggregations) is specified,
because that will be ignored on the server side in RestCountAction.

By deprecating the current setter & constructor that accept a
SearchSourceBuilder and adding replacement that accepts a QueryBuilder
it is clear what the count api can handle from HLRC side.

Follow up from #46829
martijnvg added a commit that referenced this pull request Sep 23, 2019
Currently the CountRequest accepts a search source builder, while the
RestCountAction only accepts a top level query object. This can lead
to confusion if another element (e.g. aggregations) is specified,
because that will be ignored on the server side in RestCountAction.

By deprecating the current setter & constructor that accept a
SearchSourceBuilder and adding replacement that accepts a QueryBuilder
it is clear what the count api can handle from HLRC side.

Follow up from #46829
@martijnvg martijnvg added v7.4.0 and removed v7.4.1 labels Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants