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

Add missing sum_other_doc_count to TermsAggregation #2183

Closed

Conversation

tyrantkhan
Copy link

@tyrantkhan tyrantkhan commented Jul 15, 2023

Hello There!

I've been working on moving some code from the legacy client to the java api client and noticed these two fields were missing from the spec.

btw I'm not sure there is a workaround for this currently, so i think this should be backported, but please advise if this is not right (the api is usable but if you require these fields there is no alternative)

Closes #2636

@pquentin pquentin changed the title shard_min_doc_count,sum_other_doc_count missing from Terms Aggregation Spec. Add missing sum_other_doc_count to TermsAggregation Jun 21, 2024
@pquentin
Copy link
Member

Hey @tyrantkhan, thanks for the contribution, and sorry for the delay getting back to you (nearly one year, wow). I really appreciate that you took the time to open a pull request here.

What I had done until now is to "blindly" merge main into your pull request to get up-to-date CI and code. Indeed, I merged #2175 yesterday which includes the first part of your change (shard_min_doc_count). Now, I'm reviewing the other part: sum_other_doc_count. But looking more closely, this field is set on responses, not requests, and is already correctly defined here:

As a result, there's nothing more to do here, and I'm going to close this pull request. If you have other issues with any of the Elasticsearch clients, let us know, and we'll get back to you faster next time!

@pquentin pquentin closed this Jun 21, 2024
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.

2 participants