Skip to content
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

v0.12.0: pull_blob_stream result does not implement Stream #157

Closed
burgerdev opened this issue Aug 12, 2024 · 2 comments · Fixed by #158
Closed

v0.12.0: pull_blob_stream result does not implement Stream #157

burgerdev opened this issue Aug 12, 2024 · 2 comments · Fixed by #158

Comments

@burgerdev
Copy link

The return type changed from impl Stream to SizedStream, which does not implement Stream.

Works in v0.11.0:

diff --git a/src/client.rs b/src/client.rs
index d2a0646..3b4d710 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -1006,6 +1006,27 @@ impl Client {
     ///
     /// This is a streaming version of [`Client::pull_blob`].
     /// Returns [`Stream`](futures_util::Stream).
+    ///
+    /// ```
+    /// use oci_distribution::{Client, Reference};
+    /// use oci_distribution::client::ClientConfig;
+    /// use oci_distribution::manifest::OciDescriptor;
+    /// use std::future::Future;
+    /// use std::io::Error;
+    /// 
+    /// struct StreamHandler<S: futures_util::Stream<Item = std::result::Result<bytes::Bytes, std::io::Error>>>{
+    ///   stream: S
+    /// };
+    /// 
+    /// async {
+    ///   let config = ClientConfig { ..Default::default() };
+    ///   let client = Client::new(config);
+    ///   let imgRef: Reference = "busybox:latest".parse().unwrap();
+    ///   let desc = OciDescriptor { digest: "sha256:deadbeef".to_owned(), ..Default::default() };
+    ///   let stream = client.pull_blob_stream(&imgRef, &desc).await.unwrap();
+    ///   StreamHandler{stream};
+    /// };
+    /// ```
     pub async fn pull_blob_stream(
         &self,
         image: &Reference,

Fails in v0.12.0 with the trait Stream is not implemented for SizedStream

diff --git a/src/client.rs b/src/client.rs
index 119c3e0..c4785f5 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -1091,6 +1091,27 @@ impl Client {
     ///
     /// This is a streaming version of [`Client::pull_blob`].
     /// Returns [`Stream`](futures_util::Stream).
+    ///
+    /// ```
+    /// use oci_client::{Client, Reference};
+    /// use oci_client::client::ClientConfig;
+    /// use oci_client::manifest::OciDescriptor;
+    /// use std::future::Future;
+    /// use std::io::Error;
+    /// 
+    /// struct StreamHandler<S: futures_util::Stream<Item = std::result::Result<bytes::Bytes, std::io::Error>>>{
+    ///   stream: S
+    /// };
+    /// 
+    /// async {
+    ///   let config = ClientConfig { ..Default::default() };
+    ///   let client = Client::new(config);
+    ///   let imgRef: Reference = "busybox:latest".parse().unwrap();
+    ///   let desc = OciDescriptor { digest: "sha256:deadbeef".to_owned(), ..Default::default() };
+    ///   let stream = client.pull_blob_stream(&imgRef, &desc).await.unwrap();
+    ///   StreamHandler{stream};
+    /// };
+    /// ```
     pub async fn pull_blob_stream(
         &self,
         image: &Reference,
@burgerdev
Copy link
Author

I just realized that the SizedStream.stream field is public, which then only means that the docstring needs to be adjusted.

@thomastaylor312
Copy link
Contributor

thomastaylor312 commented Aug 12, 2024

Ah yep I must have missed that in review. I thought about requiring it to impl Stream, but felt like overkill at the time (because we'd done a bunch of back and forth on the PR). I can update doc string and maybe I'll just implement stream on it while I'm there 😆

thomastaylor312 added a commit to thomastaylor312/oci-distribution that referenced this issue Aug 12, 2024
This impls a simple passthrough stream implementation for `SizedStream`.
It also updates the doc to show how you can use the stream directly.

Fixes oras-project#157

Signed-off-by: Taylor Thomas <taylor@cosmonic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants