Tasks Parity #1
Open
Tasks Parity #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
417: [1.4.0] Refactor Settings and Add Pagination, Separator and Dictionary Settings r=curquiza a=Sherlouk # Pull Request ## Related issue Fixes meilisearch#308 Fixes changes raised under meilisearch/integration-guides#286 (meilisearch#406) -⚠️ Wait for v1.4.0 ## What does this PR do? - Refactors index settings to improve reuse (functionally identical, but around 300 less lines of code) - Cleaning up some unsafe practices in unit tests - Adds support for three new types of index-level settings (improving parity) All integration tests have been run against v1.4.0 (pre-release) RC 2 and is running locally. CI uses the latest version of the executable (1.3.0) and thus will not work and will throw errors. This is expected. This PR won't be merged until after the 1.4.0 release due to Swift being a tier 3 project, as such we can simply re-run CI once that's released. This PR starts to contain a little bit of cleanup with some various nit picks (nothing that changes the code style though). ## 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: James Sherlock <[email protected]>
413: Add support for search ranking score r=curquiza a=Sherlouk # Pull Request ## Related issue Fixes meilisearch#398 ## What does this PR do? - Add the ability receive a new param in the search method called showRankingScore. - Add the ability handle the new _rankingScore key/value in the search hits' response. - Add integration tests - Update the code-samples accordingly⚠️ **This is a breaking change**. I'd love to hear some thoughts from others on how we may be able to avoid this kind of breaking change, but with meilisearch#398 and meilisearch#399 new values are being added to the "hit" object. The hit object is currently a generic provided by the user of the library, and as such it's impossible for us to add new attributes to that. In this PR I've tried to get around this by creating our own encapsulation called "SearchHit" which itself holds a reference to the actual result but also any additional attributes that Meilisearch itself returns. In order to minimise the impact on users of the library I have implemented a "dynamic member lookup" with a keypath, this continues to support (in a completely type safe and compiler safe way) dot notation to variables within the hit result. Which means `hits[0].title` would still return the title on the hit, even though architecturally it's address is `hits[0].result.title`. While this does work for the vast majority of use cases, we are unable to support the case where a user tries to cast a hit to a type, you can see this in our tests where you can no longer cast `hits[0]` to a "Book", rather it is now a "SearchHit<Book>", but again... you can access variables the same and no other changes are needed to the code. Importantly though this workaround with dynamic member lookups is limited to the capabilities of keypaths, which as documented officially by Apple [here](https://github.com/apple/swift/blob/main/test/attr/attr_dynamic_member_lookup.swift#L671-L672) does not include methods. As such where users have functions within their models, they would have to call that through `hits[0].result.someFunction()`. I do personally think this is the best compromise. It is a breaking change, but most will not be impacted in any meaningful way. If others have a better idea though I'd love to hear it. This PR lays up the foundations for meilisearch#399 which would be significantly easier once this is merged. But let's discuss. ## 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: James Sherlock <[email protected]> Co-authored-by: Clémentine U. - curqui <[email protected]>
411: Add typo-tolerance APIs r=curquiza a=Sherlouk # Pull Request ## Related issue Fixes meilisearch#282 ## What does this PR do? - Adds support for reading, updating, and resetting typo-tolerance preferences ## 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: James Sherlock <[email protected]>
This acts as an example for the conversation being held in issue meilisearch#332. Async/await continues to grow in popularity and all modern codebases are moving towards its usage. This commit demonstrates how we can add support for Swift Concurrency without breaking existing compatibility with older operating systems. For a test I have taken an existing test, copied it, and converted it for async/await. In this instance, removing the XCTestExpectation and simply using an async test available since Xcode 13 back in September 2021.
I believe as we increase this implementation to support more APIs, it will become unwieldly stuck in one file. This commit pulls it out into a separate directory for ease of maintenance.
410: Add async overload for search API r=curquiza a=Sherlouk # Pull Request⚠️ I've marked this as draft as it current works to provide an example of how we could add async/await support to the library. I am more than happy to expand this to **all** currently supported APIs. ## Related issue Relates to meilisearch#332 (doesn't close as only adds support for one API) ## What does this PR do? - Adds an async/await override to an existing search API providing users with more choice. ## 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: James Sherlock <[email protected]>
No functionality change, just an improvement to tests to prevent the test runner from crashing (and less complaints from SwiftLint)
Sometimes it would fail stating the task was still there, I believe this was because it hadn't finished deleting. Hopefully it'll now wait. Passed 50 iterations locally.
Sherlouk
force-pushed
the
36X-task-parity
branch
from
cff33b7
to
41d7976
Compare
Sherlouk
force-pushed
the
36X-task-parity
branch
from
dc3d6fc
to
f10885c
Compare
Other solutions include: - fetching all documents and asserting total count - deleting all documents before adding again (expect 8) - create a new index just for this one test
Sherlouk
force-pushed
the
36X-task-parity
branch
from
9e55019
to
4bcee63
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR has been raised against the fork until the base branch (meilisearch#417) has been merged.
Closes meilisearch#363
Closes meilisearch#366
Closes meilisearch#368
User/Client Facing
Dateobjects instead ofStringfor dates within task modelstotalto task results response (replacing Addtotalto TasksResult meilisearch/meilisearch-swift#412)try!(force try) to prevent crashes in test runnerBreaking Changes
Task.Statushas a new value.canceled(handle in anyswitchcases)TaskTypereplaces a previous String based API (use.descriptionto access String)TasksQueryuses compiled types instead of String APIs (example:"indexUpdate"is now.indexUpdate)enqueuedAtnow uses Date instead of ISO-8601 StringParity
Reviewing documentation available on MeiliSearch's website, this PR brings the
TasksAPI up to 100% parity with existing features. It resolves many errors caused by the previous design, and provides future-proofness against further scope within the tasks API.