-
Notifications
You must be signed in to change notification settings - Fork 1
First version of async traits and async HTTP #74
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
Conversation
|
@vytautas-astrauskas-sensmetry @victor-linroth-sensmetry @andrius-puksta-sensmetry I will clarify later today what would be good to review. |
My main concerns at the moment are the async versions of the traits. I did a very naive translation from synchronous constructs to
There are lots of things to improve about these traits in general (for example splitting off |
vytautas-astrauskas-sensmetry
left a comment
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.
Skimmed through. I would need to start using async more extensively to see whether the design works since I do not have much experience with it. I would suggest merging and if needed changing things later.
The only thing that I did not get is why there is much commented out code. Is all of it related to range requests?
This is still effectively a draft merge request. |
5fe26cc to
e43973e
Compare
|
Ok, this appears to be mostly working now, I will try to fix the bindings and address comments. |
Signed-off-by: Tilo Wiklund <tilo.wiklund@sensmetry.com>
53e651e to
602bab4
Compare
Signed-off-by: Tilo Wiklund <tilo.wiklund@sensmetry.com>
Signed-off-by: Tilo Wiklund <tilo.wiklund@sensmetry.com>
Signed-off-by: Tilo Wiklund <tilo.wiklund@sensmetry.com>
Co-authored-by: Andrius Pukšta <andrius.puksta@sensmetry.com> Signed-off-by: Tilo Wiklund <75035892+tilowiklundSensmetry@users.noreply.github.com>
Signed-off-by: Tilo Wiklund <tilo.wiklund@sensmetry.com>
This introduces new async traits
ProjectReadAsync,ReadEnvironmentAsync, andResolveReadAsync; mirroringProjectRead,ReadEnvironment, andResolveRead.All
reqwest-based implementations have been changed to async, usingreqwest_middleware::ClientWithMiddlewareinstead ofreqwest::blocking::Client.Utility classes such as
SequentialResolvershould now work for both synchronous and asynchoronous use, i.e.SequentialResolver<T>isResolveReadAsyncifT: ResolveReadAsyncand isResolveReadifT: ResolveRead. To the extent possible, they try to intelligentlyjoinfutures to allow concurrency.Utility classes
AsAsync[Project|Environment|Resolver]<T>andAsSync[Project|Environment|Resolver]Tokio<T>allow treating a synchronous implementation as a (trivially) asynchronous one, and to treat an asynchronous one as if it were synchronous (if provided with atokioruntime). Traits functionsto_asyncandto_sync_tokiohave been added to all the relvant traits.The CLI utilises the new async version of HTTP implementations, at the cost of having to pass around an
Arc<tokio::runtime::Runtime>. This is a bit messy, but, I think, good enough for the moment.NOTE: I have (for now) removed support for range requests when accessing kpar archives. In practice this was (at the moment) slower, and I couldn't find a readily available, simple, async way of implementing it.