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

RankingScoreThreshold PR is unsupported on older meilisearch #601

Closed
zoriya opened this issue Jan 2, 2025 · 4 comments · Fixed by #603
Closed

RankingScoreThreshold PR is unsupported on older meilisearch #601

zoriya opened this issue Jan 2, 2025 · 4 comments · Fixed by #603
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@zoriya
Copy link

zoriya commented Jan 2, 2025

Since #582 got merged, the minimum required version of meilisearch becomes v1.9.0. I'd expect older versions to still be supported if the new feature (RankingScoreThreshold in this case) is not used.

Logs

Meilisearch.MeilisearchApiError: MeilisearchApiError, Message: Unknown field `rankingScoreThreshold`: expected one of `q`, `vector`, `offset`, `limit`, `page`, `hitsPerPage`, `attributesToRetrieve`, `attributesToCrop`, `cropLength`, `attributesToHighlight`, `showMatchesPosition`, `showRankingScore`, `showRankingScoreDetails`, `filter`, `sort`, `facets`, `highlightPreTag`, `highlightPostTag`, `cropMarker`, `matchingStrategy`, `attributesToSearchOn`, Code: bad_request, Type: invalid_request, Link: https://docs.meilisearch.com/errors#bad_request
at Meilisearch.MeilisearchMessageHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
at System.Net.Http.HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
at Meilisearch.Index.SearchAsync[T](String query, SearchQuery searchAttributes, CancellationToken cancellationToken)
at Kyoo.Meiliseach.SearchManager._Search[T](String index, String query, String where, Sort`1 sortBy, SearchPagination pagination, Include`1 include) in /app/src/Kyoo.Meilisearch/SearchManager.cs:line 77
at Kyoo.Core.Api.SearchApi.SearchItems(String q, Sort`1 sortBy, Filter`1 filter, SearchPagination pagination, Include`1 fields) in /app/src/Kyoo.Core/Views/Resources/SearchApi.cs:line 156
at lambda_method1790(Closure, Object)
at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.AwaitableObjectResultExecutor.Execute(ActionContext actionContext, IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Awaited|12_0(ControllerActionInvoker invoker, ValueTask`1 actionResultValueTask)
at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeNextActionFilterAsync>g__Awaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)
at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeInnerFilterAsync>g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextExceptionFilterAsync>g__Awaited|26_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)

@curquiza
Copy link
Member

curquiza commented Jan 8, 2025

Hello here
the parameter rankingScoreThreshold is not supposed to be mandatory, and should not be sent by the SDK
I think there is issue in this file:

public decimal RankingScoreThreshold { get; set; }

Where the parameter should be optinial (?). What do you think @ahmednfwela

For me it's bug, because you should be able to use the latest version of the SDK without upgrading to v1.9.0

@curquiza curquiza added bug Something isn't working good first issue Good for newcomers labels Jan 8, 2025
@ahmednfwela
Copy link
Collaborator

@curquiza yes definitely it's a bug, it should be optional

@ahmednfwela
Copy link
Collaborator

it would be great if instead of only testing against the latest meilisearch version (

MEILISEARCH_VERSION=v1.9.0
), the integration tests would run tests against the oldest supported meilisearch version as well to avoid cases like this in the future

@curquiza
Copy link
Member

curquiza commented Jan 8, 2025

@ahmednfwela thanks for the answer

I need we test at the latest as well. But we could test both. The oldest and the latest. The problem with testing the oldest is we will have to adapt the tests because not all of them will pass (the tests will new features will not pass)

@meili-bors meili-bors bot closed this as completed in 4e4ce29 Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants