Skip to content

Errors are silent in multiGet #1678

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

Closed
efreeti opened this issue Feb 1, 2021 · 1 comment · Fixed by #1710
Closed

Errors are silent in multiGet #1678

efreeti opened this issue Feb 1, 2021 · 1 comment · Fixed by #1710
Assignees
Labels
type: enhancement A general enhancement

Comments

@efreeti
Copy link

efreeti commented Feb 1, 2021

When executing the multiGet request Elasticsearch client does provide a failure information per each entry. For some not very clear reason to me the implementation for ReactiveElasticsearchClient (and I think ElasticsearchClient too) is silenting those in the same way as non existing entries - https://github.com/spring-projects/spring-data-elasticsearch/blob/master/src/main/java/org/springframework/data/elasticsearch/client/reactive/DefaultReactiveElasticsearchClient.java#L352

It is not clear to me why this is a desired behaviour. This also makes it impossible to use multiGet from Spring Data layer and handle errors. It is also not very easy to override this behaviour by extending the class - a lot of used utility methods are private.

It would really be useful to not force this and either change the behaviour of client or at least add option to prevent this.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 1, 2021
@sothawo
Copy link
Collaborator

sothawo commented Feb 1, 2021

Best would be to return a Flux<MultiGetItemResponse> and not filtering out the failed responses and let the user filter the failed responses; at least for the ReactiveElasticsearchClient. The ReactiveElasticsearchTemplate might keep filtering these invalid values as on this level we return entities. Perhaps we need to introduce something like a GetResult<T> to model this.

The non-reactive code returns null for failed documents, which is non possible in a Flux, I think that is the reason why they were skipped in the current implementation.

@sothawo sothawo added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 1, 2021
@sothawo sothawo self-assigned this Feb 19, 2021
sothawo added a commit that referenced this issue Feb 27, 2021
Original Pull Request #1710 
Closes #1678
@sothawo sothawo added this to the 4.2 M5 (2021.0.0) milestone Feb 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants