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
Give Image::pull
, Image::build
and Image::import
chunked json a type
#262
Conversation
Image::pull
, Image::build
and Image::import
chunked json a type
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.
I like the changes a lot! Though I wish I could find the documentation for these chunks in the docker API 🙃
src/rep.rs
Outdated
#[derive(Serialize, Deserialize, Debug)] | ||
pub struct ImagePullChunk { | ||
status: String, | ||
id: Option<String>, | ||
progress: Option<String>, | ||
#[serde(rename = "progressDetail")] | ||
progress_detail: Option<ProgressDetail>, | ||
} | ||
|
||
#[derive(Serialize, Deserialize, Debug)] | ||
pub struct ProgressDetail { | ||
current: Option<u64>, | ||
total: Option<u64>, | ||
} |
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.
I feel like this would be better as an enum instead of a struct with optional fields, but I can't think of a good way to slice it...
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.
I don't think it makes sense to split it up into an enum as all fields can be present at the same time or 3 fields, or just 1
Yes, I wish it was documented somehow as well. Only way to find out what is available is to test every possibility and even then it's sometimes hard to fake an error or some random condition that would return a chunk not properly handled. As a side note, I also find it annoying that most fields in API reference, although declared as not nullable are actually nullable and one has to determine by doing lots of tests... |
src/rep.rs
Outdated
#[derive(Serialize, Deserialize, Debug)] | ||
#[serde(untagged)] | ||
/// Represents a response chunk from Docker api when building, pulling or importing an image. | ||
pub enum ImageBuildChunk { |
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.
I'm also wondering if this is the best name. Maybe ImageStreamChunk
or something else?
Ok, so perhaps not in api reference but the struct |
I'm wondering how to handle this one. Should I create EDIT: Current way definitely feels much more readable compared to this:
|
So I rebased the PR, and brought back |
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.
Cool! I guess you need to rebase here as well. 👍
What did you implement:
Added a type
ImageBuildChunk
for stream chunks when usingImage::pull
andImage::build
.Closes: #144
How did you verify your change:
Tested locally and got expected results:
Image::pull
Image::Build
What (if anything) would need to be called out in the CHANGELOG for the next release:
I updated the CHANGELOG accordingly.