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

DATAES-595 - Support for setting Elasticsearch Preference in the Quer… #288

Merged
merged 1 commit into from Jun 21, 2019

Conversation

Projects
None yet
2 participants
@fazaza
Copy link
Contributor

commented Jun 19, 2019

Support for setting Elasticsearch Preference in the Query Apis

  • You have read the Spring Data contribution guidelines.
  • There is a ticket in the bug tracker for the project in our JIRA.
  • You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.
  • You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).
@sothawo
Copy link
Collaborator

left a comment

Thank you for the contribution!
Please make sure that you complete all checks from the guidance form.

  • see my comment for the ElasticsearchTemplateTests
  • I miss a test for the changes in ElasticsearchRestTemplate
  • the preference must also be added to the SearchRequestin the ReactiveElasticSearchTemplate.doFind() method and should be tested there as well
@fazaza

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2019

Thanks for review, will update soon ...

fazaza added a commit to fazaza/spring-data-elasticsearch that referenced this pull request Jun 20, 2019

@fazaza

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2019

Please find below update :

  • ElasticsearchTemplate : add test for valid / invalid preference

  • ElasticsearchRestTemplate : already tested in super class ElasticsearchTemplateTests
    for the invalid preference test there is a different Exception type ( llegalArgumentException in case of ElasticsearchTemplate and ElasticsearchStatusException in case of ElasticsearchRestTemplate ), this is why I expect generic Exception
    let me know if you prefer to do it in another way

  • ReactiveElasticSearchTemplate : support preference and add test for valid / invalid preference

@sothawo

This comment has been minimized.

Copy link
Collaborator

commented Jun 20, 2019

I adpated the Jira issue title to "Support for setting preference parameter in query", so please squash your commits to a single one with the message "DATAES-595 - Support for setting preference parameter in query."; then I'm fine with this PR.

@fazaza fazaza force-pushed the fazaza:DATAES-595 branch from 45fec5b to 24738ca Jun 21, 2019

@fazaza

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2019

done, please review

@sothawo sothawo merged commit 40ea40a into spring-projects:master Jun 21, 2019

1 check passed

ci/pivotal-cla Thank you for signing the Contributor License Agreement!
Details
@sothawo

This comment has been minimized.

Copy link
Collaborator

commented Jun 21, 2019

the PR has been merged; thank you for the contribution!

when submitting the next PR, please check of the boxes in the guidance form at the top of the PR with x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.