-
Notifications
You must be signed in to change notification settings - Fork 58
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 multisearch to MeilisearchClient #571
base: main
Are you sure you want to change the base?
Conversation
Hello @apt-TebbeM thank you for your PR! Let me know when the PR is ready 😊 |
…e the code more readable
I think it could be reviewed. I actually thought about chaning the API for the federated search part to something like this: public async Task<ISearchable<T>> FederatedMultiSearchAsync<T>(List<FederatedSearchQuery> queries,
MultiSearchFederationOptions federationOptions = default(MultiSearchFederationOptions),
CancellationToken cancellationToken = default) to make it less likely to have an empty list of queries. |
you can go ahead |
@curquiza when will this pr be reviewed? :) |
Hello @apt-TebbeM I'm sorry but I have a loooot on my plate right now. I will maybe be able to review your PR next week. I'll do my best. But busy work period at the moment But be sure I don't forget you! |
{ | ||
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, | ||
PropertyNamingPolicy = JsonNamingPolicy.CamelCase, | ||
Converters = { new MultiSearchFederationOptionsConverter() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just add the converter as an annotation?
Pull Request
Related issue
Fixes #560
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!