search

heroku / docker-registry-client

A Go API client for the v2 Docker Registry API
BSD 3-Clause "New" or "Revised" License
297 stars 225 forks source link

Expose layer size api #12

Closed rdallman closed 8 years ago

rdallman commented 8 years ago

We are interested in getting the size of an image prior to downloading it via the registry API, so we would like to use this endpoint in order to do so since it appears to be the only documented, supported way. If anyone knows otherwise I would be just delighted to hear it. It appears at present we have to get the manifest and get the size of each layer and sum those numbers to get the total image size. This was slightly disappointing since (*SignedManifest).References() appears to return a struct that contains the size, but never appears to fill in size (nor does the manifest API return such info) -- it is not the end of the world.

This would be a breaking change to the API, as it was not documented that this client is unstable, perhaps I should assume that the API is unbreakable. I am OK with either way, personally, I just would like to add this functionality to the API any way possible but this seems ideal if API breakage is bearable. If you would prefer not to, my ideas for a non-breaking API change would be to add a method along the lines of:

func (registry *Registry) LayerExists(repository string, digest digest.Digest) (ok bool, layerSize int64, err error)

// or

func (registry *Registry) LayerMetadata(repository string, digest digest.Digest) (descriptor.Descriptor, err error)

I am open to any other ideas, of course. Please let me know your wishes :)

Thank you for building this and contributing it back to the community!

ojacobson commented 8 years ago

I like the implementation, though I am surprised by the underlying context.

We haven't nailed down a versioning policy for this, yet, but it's in production use in at least a couple of places, so I'd prefer a non-breaking API. A struct-returning variation on this is more forwards-compatible - would you be willing to put together the func (registry *Registry) LayerMetadata(repository string, digest digest.Digest) (descriptor.Descriptor, err error) variation?

rdallman commented 8 years ago

@ojacobson thanks for the speedy review and feedback. I've updated the code https://github.com/heroku/docker-registry-client/pull/12/commits/e3ef7b4c15a729f13de754a9f2441c1853eca801 to use distribution.Descriptor (this is what I meant by descriptor.Descriptor but, sigh, I am faulty). Please let me know what you think. Thanks!

edit: for some context, I took this idea from digging around code here -- I'm unsure whether I like adding the media type since different manifest versions could change it and this API doesn't tell us which, we could, alternatively, use a different struct.

ojacobson commented 8 years ago

💥 Thanks! That looks great. I've merged it in!

rdallman commented 8 years ago

thanks!