Skip to content

Rank feature - unknown field linear #983

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 29, 2021
Merged

Rank feature - unknown field linear #983

merged 1 commit into from
Jul 29, 2021

Conversation

Hronom
Copy link
Contributor

@Hronom Hronom commented Jul 18, 2021 •
edited
Loading

Description

Add support for rank feature query linear

Issues Resolved

#977

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success f76ebaa

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed f76ebaa

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success f76ebaa

@setiah
Copy link
Contributor

setiah commented Jul 22, 2021

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success f76ebaa
Log 344

Reports 344

@dblock
Copy link
Member

dblock commented Jul 22, 2021 •
edited
Loading

Much thanks!

@nknize Care to double check for me that this looks right before/and merge?

@Hronom We need to document this - care to PR or open an issue in https://github.com/opensearch-project/documentation-website please?

@dblock
Copy link
Member

dblock commented Jul 22, 2021

@Hronom How do you feel about adding a REST API-level test for this that matches what you described in #977?

dblock
dblock suggested changes Jul 22, 2021
View reviewed changes
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

I clicked approve too fast, I'll get back to this.

@Hronom
Copy link
Contributor Author

Hronom commented Jul 23, 2021

@dblock I will try to put some docs on weekends

@Hronom
Copy link
Contributor Author

Hronom commented Jul 23, 2021 •
edited
Loading

@dblock I pretty new in this project, can you help me:

@Hronom How do you feel about adding a REST API-level test for this that matches what you described in #977?

Where is this REST API-level tests? Please point me to the class or example of it. I added tests for this feature, but not sure is they API level or not...

@Hronom
Copy link
Contributor Author

Hronom commented Jul 24, 2021

@dblock added docs opensearch-project/documentation-website#114

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success e02324ec445460bef706939b9e1d8bb811c85716

@opensearch-ci-bot
Copy link
Collaborator

❌   DCO Check Failed e02324ec445460bef706939b9e1d8bb811c85716
Run ./dev-tools/signoff-check.sh remotes/origin/main e02324ec445460bef706939b9e1d8bb811c85716 to check locally
Use git commit with -s to add 'Signed-of-by: {EMAIL}' on impacted commits

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success e02324ec445460bef706939b9e1d8bb811c85716

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success f76ebaa

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed f76ebaa

@Hronom
Copy link
Contributor Author

Hronom commented Jul 25, 2021

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success f76ebaa

@dblock
Copy link
Member

dblock commented Jul 26, 2021

I didn't forget about this, please bear with me. I need to do some homework before this can be merged, related to the fork.

@dblock
Copy link
Member

dblock commented Jul 28, 2021

@Hronom Thank you for your contribution! This PR looks very similar to elasticsearch#67438 and elasticsearch#67670. Was your code based off either of those PRs, or was it based on the current master of elasticsearch?

@Hronom
Copy link
Contributor Author

Hronom commented Jul 28, 2021 •
edited
Loading

@dblock It's based on that PR's with small addition according to stile of other functions in OpenSearch, also it use function provided by core Lucene.

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed ea29c70a813a4eceefb888caa34969aafff5a1a9

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success ea29c70a813a4eceefb888caa34969aafff5a1a9

@Hronom
Rank feature - unknown field linear
e8b2013 
Signed-off-by: Yevhen Tienkaiev <[email protected]>
@Hronom
Copy link
Contributor Author

Hronom commented Jul 28, 2021

Added fix in exception message for supported functions

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success e8b2013

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed e8b2013

@Hronom
Copy link
Contributor Author

Hronom commented Jul 28, 2021

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success ea29c70a813a4eceefb888caa34969aafff5a1a9

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success e8b2013

@Hronom Hronom requested a review from dblock July 28, 2021 23:26
@dblock
Copy link
Member

dblock commented Jul 29, 2021

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success e8b2013
Log 359

Reports 359

@dblock dblock merged commit f46f695 into opensearch-project:main Jul 29, 2021
@dblock
Copy link
Member

dblock commented Jul 29, 2021

Merged, thank you @Hronom. I imagine we want this in 1.1. Could you please open a backport PR into that branch? Thank you.

@dblock dblock added the pending backport Identifies an issue or PR that still needs to be backported label Jul 29, 2021
@Hronom Hronom mentioned this pull request Jul 29, 2021
Merged
5 tasks
@Hronom
Copy link
Contributor Author

Hronom commented Jul 29, 2021

@dblock created #1025

dblock pushed a commit that referenced this pull request Jul 30, 2021
@Hronom
Rank feature - unknown field linear (#983) (#1025)
a936e1e 
Signed-off-by: Yevhen Tienkaiev <[email protected]>
@dblock dblock removed the pending backport Identifies an issue or PR that still needs to be backported label Jul 30, 2021
@tlfeng tlfeng added v1.1.0 Issues, PRs, related to the 1.1.0 release v2.0.0 Version 2.0.0 enhancement Enhancement or improvement to existing feature or request labels Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dblock dblock dblock approved these changes

Assignees
No one assigned
Labels
enhancement Enhancement or improvement to existing feature or request v1.1.0 Issues, PRs, related to the 1.1.0 release v2.0.0 Version 2.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Hronom @opensearch-ci-bot @setiah @dblock @tlfeng