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

Add federated search #393

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

ellnix
Copy link
Collaborator

@ellnix ellnix commented Feb 6, 2025

Pull Request

Related issue

Continues on top of #384
Fixes #389

I've got the general idea of the implementation, what's left is:

  • Finish writing tests
  • Add to README
  • Refactor (especially warnings and error handling)
  • Investigate pagination

Ready for review

@ellnix ellnix added the enhancement New feature or request label Feb 6, 2025
@ellnix ellnix self-assigned this Feb 6, 2025
@ellnix ellnix force-pushed the federated-search branch 2 times, most recently from 0a09b55 to 78d662d Compare February 7, 2025 21:25
Copy link

codecov bot commented Feb 7, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.38%. Comparing base (b5d918f) to head (d2ab989).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #393      +/-   ##
==========================================
+ Coverage   89.56%   90.38%   +0.81%     
==========================================
  Files          13       14       +1     
  Lines         776      842      +66     
==========================================
+ Hits          695      761      +66     
  Misses         81       81              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ellnix ellnix marked this pull request as ready for review February 7, 2025 21:30
@ellnix ellnix requested a review from brunoocasali February 7, 2025 21:30
@ellnix ellnix changed the title WIP: Add federated search Add federated search Feb 7, 2025
@wesharper
Copy link

My team has been using the forked repo's branch in our staging environment and everything is working smoothly. We will push to prod soon, but would love to see this hit the mainline release pipeline before pulling the trigger in an ideal world.

@brunoocasali
Copy link
Member

I will review this after the rebase or the merge of the class name change :)

@ellnix ellnix force-pushed the federated-search branch from 1e3c302 to d4080ed Compare March 19, 2025 14:29
@@ -11,23 +12,59 @@ def multi_search(searches)
normalize(options, index_target)
end

MultiSearchResult.new(searches, client.multi_search(search_parameters))
MultiSearchResult.new(searches, client.multi_search(queries: search_parameters))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This change is necessary due to this change in ms-ruby: meilisearch/meilisearch-ruby@055f7d0

@ellnix ellnix force-pushed the federated-search branch from c2ddbab to af0c198 Compare March 19, 2025 14:50
No soft deprecation here, since federated search has not been published
yet.
@ellnix ellnix force-pushed the federated-search branch from 5a0b981 to d2ab989 Compare March 19, 2025 15:18
@ellnix
Copy link
Collaborator Author

ellnix commented Mar 19, 2025

@brunoocasali Rebased and also removed :class_name option similar to #405

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support federated search
3 participants