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

Remove Blob conversion from ImageToTarballTranslator and TarStreamBuilder #1647

Merged
merged 7 commits into from
Apr 24, 2019

Conversation

chanseokoh
Copy link
Member

@chanseokoh chanseokoh commented Apr 19, 2019

I am going to do a serious of refactoring to our repo (not about our API visibility but about making our codebase healthy), and this is the first step. I have many more PRs to come.

When I first came to the repo, Blob was a very confusing class. As much as the name says, it is a very general class and can wrap up any object, anything that holds any information, or anything that may potentially be written to somewhere. It often disguises the actual object to the point of not being able to get what it really is. To me, often Blob looks like the Java Object class.

Another problem is the abuse of Blob to write something to OutputStream. With Blob having toWrite(OutputStream), examples of prevalent abuse in our codebase are

  1. Convert anything to Blob blindly for the sake of writing to OutputStream.
  2. Convert anything to Blob blindly to compute a digest of a content. (Blob.writeTo() returns a digest.)

Currently, ImageToTarballTranslator (which effectively represents a tarball for an Image) is doing a convoluted way of writing it to a file or a Docker daemon:

ImageToTarballTranslator --> Blob --> write to an output stream

In this PR, I'm making this straightforward and intuitive by

  1. just making ImageToTarballTranslator write to an output stream; and
  2. decoupling Blob from ImageToTarballTranslator.

Also, DockerClient.load() accepting Blob seems is too general and obscure, which doesn't look very different from load(Object anything).

@chanseokoh chanseokoh changed the title Cleanup Remove Blob conversion from ImageToTarballTranslator and TarStreamBuilder Apr 19, 2019
@chanseokoh
Copy link
Member Author

Similarly to ImageToTarballTranslator, I also removed Blob from TarStreamBuilder, which it had toBlob() only to do toBlob().writeTo(out).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants