Add async_read support for pull_blob API #54
Conversation
arronwy
requested review from
thomastaylor312,
bacongobbler and
flavio
as code owners
This API allows blobs input stream integrate seamless with following blob processing using Rust's asynchronous IO types like async_compression. Fixed: oras-project#53 Signed-off-by: Wang, Arron <arron.wang@intel.com>
What's Changed ============== * Bump actions/checkout from 3.0.2 to 3.1.0 by @dependabot in oras-project#47 * feat: enable Clone, Eq, PartialEq and Debug macro for RegistryAuth by @Xynnn007 in oras-project#48 * fix: address broken test by @flavio in oras-project#52 * Update rstest requirement from 0.15.0 to 0.16.0 by @dependabot in oras-project#55 * Add async_read support for pull_blob API by @arronwy in oras-project#54 * fix: rustls allow usage or system root certificates by @flavio in oras-project#56 == New Contributors * @Xynnn007 made their first contribution in oras-project#48 Signed-off-by: Flavio Castelli <fcastelli@suse.com>
|
@arronwy I've just published v0.9.4 that features your patch |
Thanks a lot for your quick review and new v0.9.4 release, much appreciated! |
|
@flavio Not a big deal for something this small, but generally a feature add should probably be a new minor release rather than patch (so 0.10.0) |
| &self, | ||
| image: &Reference, | ||
| digest: &str, | ||
| ) -> Result<impl AsyncRead + Unpin> { |
There was a problem hiding this comment.
I know this is already merged but @arronwy is there a reason you chose AsyncRead over returning Stream? Generally it is better to return a stream and let the user choose how they want to consume it
There was a problem hiding this comment.
Hi @thomastaylor312 ， yes, agree, Stream can easy convert to AsyncRead I also verified if we return
Result<impl Stream<Item = std::result::Result<bytes::Bytes, std::io::Error>>>
will also works.
The reason I chose AsyncRead is try to consistent with current pull_blob API with output using:
AsyncWrite and async_compression crates: https://docs.rs/async-compression/latest/async_compression/index.html also operate over AsyncWrite and AsyncBufRead streams.
It's ok for me if we decide change to return Stream to let user to choose :-).
There was a problem hiding this comment.
Right I had forgotten we used AsyncRead. I'll capture an issue to refactor that at some point
Add async_read support for pull_blob API
What's Changed ============== * Bump actions/checkout from 3.0.2 to 3.1.0 by @dependabot in #47 * feat: enable Clone, Eq, PartialEq and Debug macro for RegistryAuth by @Xynnn007 in #48 * fix: address broken test by @flavio in #52 * Update rstest requirement from 0.15.0 to 0.16.0 by @dependabot in #55 * Add async_read support for pull_blob API by @arronwy in #54 * fix: rustls allow usage or system root certificates by @flavio in #56 == New Contributors * @Xynnn007 made their first contribution in #48 Signed-off-by: Flavio Castelli <fcastelli@suse.com>
This API allows blobs input stream integrate seamless with following blob processing using Rust's asynchronous IO types like async_compression.
Fixed: #53
Signed-off-by: Wang, Arron arron.wang@intel.com