Add async for I/O calls #372
Conversation
KendallWeihe
requested review from
nitro-neal,
shamilovtim,
diehuxx,
frankhinek and
leordev
as code owners
| @@ -1,4 +1,5 @@ | |||
| use crate::errors::Result; | |||
| use futures::executor::block_on; | |||
There was a problem hiding this comment.
why futures::executor::block_on vs alternatives like tokio::task::spawn_blocking?
There was a problem hiding this comment.
futures has a slightly more streamlined error handling, whereas with tokio I would have to add a mapper into our error type, see the unwrap below
pub fn resolve(uri: &str) -> Self {
let inner_result = task::spawn_blocking(move || {
tokio::runtime::Handle::current().block_on(InnerResolutionResult::resolve(uri))
}).unwrap(); // Handle unwrap or any error handling as required.
Self(inner_result)
}There was a problem hiding this comment.
Weird, I guess uniffi requires tokio? I could've sworn I tested with futures, but oh well, guess I have to go the tokio route
|
| @@ -7,6 +7,7 @@ repository.workspace = true | |||
| license-file.workspace = true | |||
|
|
|||
| [dependencies] | |||
| futures = "0.3.30" | |||
There was a problem hiding this comment.
When we chatted at standup, you were getting an error similar to this mozilla/uniffi-rs#2223. The solution may be that we have to specify async_runtime = tokio using uniffi proc macros as described here mozilla/uniffi-rs#2219.
There was a problem hiding this comment.
indeed! good catch, we can improve this later
| Method::Get => "GET".to_string(), | ||
| Method::Post => "POST".to_string(), | ||
| Method::Put => "PUT".to_string(), | ||
| impl fmt::Display for Method { |
There was a problem hiding this comment.
did you mean to change the name?
There was a problem hiding this comment.
Yeah I learned that implementing Display is slightly preferable to implementing ToString because implementing Display also implements ToString plus you get format!() support
No description provided.