Skip to content

Conversation

@wking
Copy link
Contributor

@wking wking commented Mar 1, 2018

@mikebrow and I have had a lengthy discussion before realizing that we were not interpreting these terms the same way. This commit replaces some explicit object-type lists with “objects defined in the image specification”, which is more concrete than using terms without local definitions. I've also added image-spec links where I do use object-type terms. And I've used the wordy but more explicit “groups of image indexes” instead of “repositories” in most cases.

I've also explicitly listed /v2/_catalog as out of scope. It's a higher-level endpoint than the image-spec objects. As Patrick and Stephen pointed out when the endpoint landed, it's for internal administration. Matt suggested keeping it out of the user-facing API on those grounds, and while that didn't happen in
docker/distribution, I think we want to keep the endpoint out of our distribution spec in order to avoid burdening implementors (because as Patrick pointed out, it's an expensive endpoint unrelated to image push/pull).

@wking wking force-pushed the image-repository-wording branch from bb4908e to 8dd6a19 Compare March 1, 2018 20:38
@wking wking mentioned this pull request Mar 1, 2018
@wking wking force-pushed the image-repository-wording branch 2 times, most recently from d00d12a to c2f052d Compare March 1, 2018 20:41
Copy link
Member

@mikebrow mikebrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@wking
Copy link
Contributor Author

wking commented Mar 1, 2018

ping @caniszczyk, now that @mikebrow and I are on the same page ;).

Docker's registery API already provides endpoints for fetching manifest objects by digest][get-manifest].
Docker's registery API does not currently provide endpoints for fetching image objects by digest, but this is the project where that will happen.
* “Retrieving image content by its content-addressable hash”.
Docker's registery API already provides [endpoints for fetching manifest objects by digest][get-manifest].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: s/registery/registry/ here and below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: s/registery/registry/ here and below.

Thanks; fixed with c2f052d75ede78.

Other tag-listing endpoints needed for backwards-compatibility are therefore in scope as well.

Repository actions and listing images within a repository are considered part of distribution policy or content management and are out of scope for this entry's per-image action.
Grouping image indexes in repositories is considered part of distribution policy or content management and are out of scope for this entry's per-image action.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: "Grouping image indexes ... is out of scope..."

or even:

"...is considered part of distribution policy or content management, which are out of scope..."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

…content management, which are…

Thanks, went with this form in c2f052d75ede78.

Mike and I have had a lengthy discussion before realizing that we were
not interpreting these terms the same way [1].  This commit replaces
some explicit object-type lists with "objects defined in the image
specification", which is more concrete than using terms without local
definitions.  I've also added image-spec links where I do use
object-type terms.  And I've used the wordy but more explicit "groups
of image indexes" instead of "repositories" in most cases.

I've also explicitly listed /v2/_catalog as out of scope.  It's a
higher-level endpoint than the image-spec objects.  As Patrick [2] and
Stephen [3] pointed out when the endpoint landed, it's for internal
administration.  Matt suggested keeping it out of the user-facing API
on those grounds [4], and while that didn't happen in
docker/distribution, I think we want to keep the endpoint out of our
distribution spec in order to avoid burdening implementors (because as
Patrick pointed out [2], it's an expensive endpoint unrelated to image
push/pull).

[1]: opencontainers#44 (comment)
[2]: distribution/distribution#653 (comment)
[3]: distribution/distribution#653 (comment)
[4]: distribution/distribution#653 (comment)
@wking wking force-pushed the image-repository-wording branch from c2f052d to 75ede78 Compare March 1, 2018 21:07
@caniszczyk
Copy link
Contributor

LGTM thanks

@caniszczyk caniszczyk merged commit f92fdb3 into opencontainers:add-distribution-proposal Mar 1, 2018
@wking wking deleted the image-repository-wording branch April 4, 2018 23:18
bsatlas added a commit to bsatlas/distribution-spec that referenced this pull request Jan 4, 2019
bsatlas added a commit to bsatlas/distribution-spec that referenced this pull request Jan 4, 2019
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 this pull request may close these issues.

4 participants