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

Implement vector search experimental feature v2 (v1.6) #924

Merged
merged 14 commits into from
Feb 8, 2024

Conversation

CaroFG
Copy link
Contributor

@CaroFG CaroFG commented Feb 6, 2024

Pull Request

Related issue

Fixes #901

What does this PR do?

  • Creates embedders classes
  • Adds embedders to paths
  • Introduces new routes:
    • Create a new method to get the settings by calling GET /indexes/:index_uid/settings/embedders
    • Create a new method to update the settings by calling PATCH /indexes/:index_uid/settings/embedders
    • Create a new method to reset the settings by calling DELETE /indexes/:index_uid/settings/embedders
  • Adds embedders settings tests
  • Updates vector search tests

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!

Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

thank you caro

Before I review, I see you missed part of the issue

Can you update it?
Capture d’écran 2024-02-06 à 15 48 44

@curquiza curquiza added enhancement New feature or request breaking-change The related changes are breaking for the users labels Feb 6, 2024
@curquiza
Copy link
Member

curquiza commented Feb 6, 2024

Putting breaking label so that the vector search users will be aware there are breaking changes on the Meilisearch side (for the experimental feature)

Copy link
Collaborator

@sanders41 sanders41 left a comment

Choose a reason for hiding this comment

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

The warning about not being compatable with vector search can also be removed from the README with this.

@CaroFG CaroFG requested review from curquiza and sanders41 February 7, 2024 10:49
Copy link
Collaborator

@sanders41 sanders41 left a comment

Choose a reason for hiding this comment

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

It's totally fine to put the new_embedders fixture in conftest.py so no need to move it back. Just as an FYI, the reason pylint was giving an error is because pylint is wrong 😄. That is why we have # pylint: disable=redefined-outer-name at the top of some other test files.

@CaroFG CaroFG requested a review from sanders41 February 7, 2024 11:36
Copy link
Collaborator

@sanders41 sanders41 left a comment

Choose a reason for hiding this comment

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

bors merge

meili-bors bot added a commit that referenced this pull request Feb 7, 2024
924: Implement vector search experimental feature v2 (v1.6) r=sanders41 a=CaroFG

# Pull Request

## Related issue
Fixes #901 

## What does this PR do?
- Creates embedders classes
- Adds embedders to paths
 - Introduces new routes:
   - Create a new method to get the settings by calling GET /indexes/:index_uid/settings/embedders
   - Create a new method to update the settings by calling PATCH /indexes/:index_uid/settings/embedders
   - Create a new method to reset the settings by calling DELETE /indexes/:index_uid/settings/embedders
  - Adds embedders settings tests
  - Updates vector search tests
 

## PR checklist
Please check if your PR fulfills the following requirements:
- [x] Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
- [x] Have you read the contributing guidelines?
- [x] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to Meilisearch!


Co-authored-by: CaroFG <[email protected]>
Co-authored-by: CaroFG <[email protected]>
Copy link
Contributor

meili-bors bot commented Feb 7, 2024

This PR was included in a batch that successfully built, but then failed to merge into main. It will not be retried.

Additional information:

{"message":"1 review requesting changes and 1 approving review by reviewers with write access.","documentation_url":"https://docs.github.com/articles/about-protected-branches"}

@sanders41
Copy link
Collaborator

sanders41 commented Feb 7, 2024

@curquiza do you know why bors would fail? When I look it say everything succeeded.

Edit: bors added a comment and it looks like it wants you to approve also.

Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

yes thank you caro! sorry for the delay

bors merge

Copy link
Contributor

meili-bors bot commented Feb 8, 2024

@meili-bors meili-bors bot merged commit bf0aea5 into meilisearch:main Feb 8, 2024
11 checks passed
CommanderStorm added a commit to CommanderStorm/meilisearch-rust that referenced this pull request Mar 8, 2024
CommanderStorm added a commit to CommanderStorm/meilisearch-rust that referenced this pull request Mar 11, 2024
CommanderStorm added a commit to CommanderStorm/meilisearch-rust that referenced this pull request Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change The related changes are breaking for the users enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v1.6] Implement vector search experimental feature
3 participants