meilisearch-swift
meilisearch-swift copied to clipboard
Add async/await support
Description Since server-side Swift projects like Vapor now support async/await APIs, it would nice if meilisearch-swift also added this.
Basic example As opposed to the completion parameter style currently used, an async/await call would look like so:
let result = await index.updateDocuments(documents: [/** data **/], primaryKey: "id")
switch result {
case .success:
// ....
case .failure(let error):
// ....
}
Proposed solution
I have already created extensions in my local project to support some of the methods that I use. For example updateDocuments()
is covered by this implementation:
public func updateDocuments<T>(
documents: [T],
encoder: JSONEncoder? = nil,
primaryKey: String? = nil) async -> Result<TaskInfo, Swift.Error> where T: Encodable {
return await withCheckedContinuation { continuation in
self.updateDocuments(documents: documents, encoder: encoder, primaryKey: primaryKey) { res in
continuation.resume(returning: res)
}
}
}
Proposal is to add these bridging methods for all methods on Indexes
in perhaps an Indexes+async.swift
extension. Additionally unit tests will need to be added to cover async calls.
I am happy to implement this, but before I do so wanted to get some input on (a) whether this feature has as much merit as I think it does (b) whether the proposed solution is acceptable.
Note that if implemented all new and updated methods on Indexes
will need to be mirrored to its async counterpart which adds a bit of overhead to maintenance.
Hi @aronbudinszky!
I'm not a Vapor
expert, but I can see good potential with this feature you're proposing. I think it could be nice to adopt.
Also, I suggest you start very small for one or two methods, and then we can iterate in the code reviews. This way, you don't invest that much time upfront, and it will be easier for me to review :)
The integrations team does not have the resources to implement or review that. If someone wants that and could ensure through tests, it does work, we can merge that.
Until then, I'm going to close this to clean the repository.
Hi @brunoocasali, appreciate your last comment. New to the project 👋
Async/Await continues to grow in it's usage across the Swift ecosystem, be it with server-side Swift (and Vapor as raised above) but even just in iOS itself and other Apple platforms. SwiftUI for example continues to implement more and more async functionality built in.
I've actually just spent an hour throwing together a Vapor service for this library, and it's quickly obvious how the lack of async/await functions makes it more difficult to utilise this with a modern codebase.
The use of "withCheckedContinuation" is a common pattern within libraries which want to maintain backwards support but still have a view for the future. Apple themselves utilise this in their open-source packages such as swift-nio. I have no doubt that the solution mentioned in the thread would absolutely work.
I'm going to open a PR tomorrow which demonstrates this for you on a single API call. I'd love to see this moved forward.
I for one am happy to contribute. Haven't had the time to add this so far. But as @sherlouk mentioned - it is more and more important.
Great to hear from you @aronbudinszky! By no means wanting to 'steal' this from you, but I have opened a PR (https://github.com/meilisearch/meilisearch-swift/pull/410) to demonstrate your idea.
If time is difficult for you, and with the blessing from Bruno, I'm happy to flesh the PR out to cover all currently supported APIs. Async/await is really important for my use cases 😄
Reopening since it looks like community has taken it again 😄
Thank you for re-opening Clem.
@aronbudinszky, as you voiced an opinion, would you mind reviewing PR #410? It would be nice to see what another user of the package thinks.
While this would introduce a lot of extra functions, and definitely increase the maintenance overhead, I think the value of such a feature to users (especially in modern applications) makes this necessary.
I'd be happy to scale up and support the maintenance of it if we're in agreement.
There's a potential solution too in the future if we upgrade the Swift version to v5.9 we could use macros to automatically generate the async/await versions. I think this is too big a leap in support to action right now (it was only released this week) but something to consider down the line.
@aronbudinszky, as you voiced an opinion, would you mind reviewing PR #410? It would be nice to see what another user of the package thinks.
Would love to have the review of the community on this. I'm not a swift developer (we don't have any at Meili), but I'm the only one who has the time to take care of the minimum maintenance of this repo. So, I can bring the Meilisearch knowledge you need 😄
I added a review just now, apologies for the delay. There's only one minor request but other than that the solution is perfect.
...and thanks @Sherlouk for pushing this forward!
I'm happy to review any further changes and in a week or two I should have some time to assist in adding async support to other functions.