Skip to content

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

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 14 commits into from
Feb 8, 2024
Merged

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

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!

curquiza
curquiza requested changes Feb 6, 2024
View reviewed changes
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)

sanders41
sanders41 requested changes Feb 6, 2024
View reviewed changes
Copy link
Collaborator

@sanders41 sanders41 left a comment •
edited
Loading

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
sanders41
sanders41 requested changes Feb 7, 2024
View reviewed changes
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
sanders41
sanders41 approved these changes Feb 7, 2024
View reviewed changes
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
@meili-bors @CaroFG
Merge #924
03ef6cf 
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 <carolina.ferreira131@gmail.com>
Co-authored-by: CaroFG <48251481+CaroFG@users.noreply.github.com>
@meili-bors
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 •
edited
Loading

@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.

curquiza
curquiza approved these changes Feb 8, 2024
View reviewed changes
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

@meili-bors
Copy link
Contributor

meili-bors bot commented Feb 8, 2024

Build succeeded:

@meili-bors meili-bors bot merged commit bf0aea5 into meilisearch:main Feb 8, 2024
CommanderStorm added a commit to CommanderStorm/meilisearch-rust that referenced this pull request Mar 8, 2024
@CommanderStorm
Added the same settings operations found in meilisearch/meilisearch-p…
1076241 
…ython#924
@CommanderStorm CommanderStorm mentioned this pull request Mar 8, 2024
Merged
4 tasks
CommanderStorm added a commit to CommanderStorm/meilisearch-rust that referenced this pull request Mar 11, 2024
@CommanderStorm
Added the same settings operations found in meilisearch/meilisearch-p…
8ffb555 
…ython#924
CommanderStorm added a commit to CommanderStorm/meilisearch-rust that referenced this pull request Apr 15, 2024
@CommanderStorm
Added the same settings operations found in meilisearch/meilisearch-p…
51820a9 
…ython#924
@Mubelotix Mubelotix mentioned this pull request Jul 10, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@curquiza curquiza curquiza approved these changes

@sanders41 sanders41 sanders41 approved these changes

Assignees

No one assigned

Labels

breaking-change The related changes are breaking for the users enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[v1.6] Implement vector search experimental feature

3 participants

@CaroFG @curquiza @sanders41