Fetch blob as stream #224
Fetch blob as stream #224
Conversation
src/v2/content_digest.rs
Outdated
| pub fn try_verify(&self, input: &[u8]) -> std::result::Result<(), ContentDigestError> { | ||
| let hash = self.algorithm.hash(input); | ||
| let layer_digest = Self::try_new(hash)?; | ||
| pub fn hash(&mut self, input: &[u8]) { |
There was a problem hiding this comment.
Not sure what this function does - if it just updates self.algorithm lets rename it to smth like detect_hash_type?
There was a problem hiding this comment.
I would rename it to update - it is well known name when hashing data.
src/v2/content_digest.rs
Outdated
| /// ContentDigest stores a digest and its DigestAlgorithm | ||
| #[derive(Clone, Debug)] | ||
| pub struct ContentDigest { | ||
| expected_digest: String, |
There was a problem hiding this comment.
Not sure if it needs to be renamed, its unclear what "expected" means here
src/v2/content_digest.rs
Outdated
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| impl std::fmt::Display for ContentDigest { | ||
| fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { | ||
| write!(f, "{}:{}", self.algorithm, self.digest) | ||
| write!(f, "{}:{}", self.algorithm, "self.digest") |
There was a problem hiding this comment.
I don't think quotes are necessary - typo?
src/v2/content_digest.rs
Outdated
| @@ -117,25 +139,39 @@ mod tests { | |||
| } | |||
|
|
|||
| #[test] | |||
| fn try_verify_succeeds_with_same_content() -> Fallible<()> { | |||
| fn verify_content_ok() -> Fallible<()> { | |||
There was a problem hiding this comment.
| fn verify_content_ok() -> Fallible<()> { | |
| fn verify_succeeds_with_same_content() -> Fallible<()> { |
so that we keep in line with verify_fails_with_different_content
|
Looks good! Lets add some unit tests - i.e. vrutkovs@325e87b |
Oh, thanks. Yesterday also wanted to add tests but didn't find out why network tests failed. |
|
Network tests are passing, but in Github Actions we get 429 from quay. Investigating this in #222 |
|
@vrutkovs We want to display a progress bar while downloading a layer. At the moment we cannot do that as there is no api to know layer's size. I suggest to slightly change a fetching blob api like following: let blob_resp = dkclient.get_blob_response().await?;
let blob_size = blob_resp.size();
let blob_stream = blob_resp.stream();What do you think about it? |
|
Sure, that looks useful |
anti-social
force-pushed
the
blob-stream
branch
2 times, most recently
from
8d4acca to
6bb2547
Compare
|
Implemented getting blob size when using stream api. |
|
This looks great, thank you! Lets merge this now and adjust later if necessary |
Here we need this functionality: tailhook/vagga#572