layer: wordsmith non-distributable section #483
Conversation
| [Descriptors](descriptor.md) referencing these layers MAY include `urls` for downloading these layers. | ||
| It is implementation-defined whether or not implementations upload layers tagged with this media type. | ||
| Non-distributable layers SHOULD be tagged with an alternative mediatype of `application/vnd.oci.image.layer.nondistributable.v1.tar+gzip`. | ||
| Implementations SHOULD NOT upload layers tagged with this media type; however, such a media type SHOULD NOT affect whether an implementation downloads the layer. |
There was a problem hiding this comment.
In a number of situations (e.g. dealing with a local cache or proxy) “downloading” and “uploading” become hard to distinguish from “internal handling” (which is why we went with the less specific approach in the first place). Do we want to provide guidelines on how we define those terms? Or is the existing “never uploaded” (which I still don't like) line sufficient precedence for “folks dealing with local caches and proxies are allowed to pick their own definition of ‘uploaded’ and we'll never actually attempt to validate this SHOULD NOT”?
|
I like (and don't see anything controversial in) everything about
a2dda98 except for the new SHOULD NOT's reliance on fuzzy “upload” and
“download” wording [1]. Is there any chance of splitting that off
into it's own PR? Then we can land the cleaner wording quickly while
we hash out whether we need to define upload/download. Unless I'm the
only person who sees uncertainty around what upload/download mean in
the cache/proxy context ;).
[1]: https://github.com/opencontainers/image-spec/pull/483/files#r91417509
|
|
I'm not sure what you're suggesting - just strike the line here for now? |
|
Yeah, drop your new "Implementations SHOULD NOT upload..." line and keep the old "It is implementation-defined..." line. Then we land this one quick and easy, and you open a new PR with just that "implementation-defined" -> "SHOULD NOT" line swap. |
| Non-distributable layers SHOULD be tagged with an alternative mediatype of `application/vnd.oci.image.layer.nondistributable.v1.tar+gzip`. | ||
| Implementations SHOULD NOT upload layers tagged with this media type; however, such a media type SHOULD NOT affect whether an implementation downloads the layer. | ||
|
|
||
| [Descriptors](descriptor.md) referencing non-distributable layers MAY include `urls` for downloading these layers directly. |
There was a problem hiding this comment.
If the
urlsfield is present, they SHOULD be favored for retrieval over resolving through thedigestidentifiers.
The presence of theurlsfield MUST NOT be used to determine whether or not a layer is non-distributable.
There was a problem hiding this comment.
If the
urlsfield is present, they SHOULD be favored for retrieval over resolving through thedigestidentifiers.
I don't think we want this, since it sounds like “don't cache these locally”. I don't think we can make generic calls about whether a digest-based retrieval will be faster or slower than urls based retrieval, and I don't see any user-side metrics for deciding between them other than speed.
The presence of the
urlsfield MUST NOT be used to determine whether or not a layer is non-distributable.
I'm fine with this.
There was a problem hiding this comment.
I don't think we want this, since it sounds like “don't cache these locally”.
Only if you read into it too much and skip words like "retrieval" and "resolving" when trying to understand the meaning. It is also a SHOULD.
There was a problem hiding this comment.
Does “resolving through the digest identifiers” somehow not include fetching from a local cache? If so, I think it's worth spelling that out explicitly (but defining “a local cache” might be tricky).
And I don't think the SHOULD-ness means we can be sloppy about defining the relevant terms. SHOULD just means compliant implementations can skip the requirement; it shouldn't mean that the requirement itself is ambiguous.
There was a problem hiding this comment.
@wking More "precision" here is going to infer a hierarchy, which actually sacrifices accuracy. The term "retrieval" implies relative displaced locality. In this context, since we are talking about urls, the term is both accurate and precise, since the relative locality is urls, provided in the first clause, which implies network access. Local cache clearly does not fall into the locality level of urls.
Adding more here will just obscure the intent of the statement. It is neither sloppy nor imprecise and ultimately accurate in context.
But let's see what the contributor has to say, as he might have an opinion.
There was a problem hiding this comment.
I don't understand why we need to call out urls as being preferential - what's wrong with the digest resolving?
There was a problem hiding this comment.
The presence of the urls field MUST NOT be used to determine whether or not a layer is non-distributable.
Mixing SHOULDs and MUSTs here is awkward. With the current wording I really don't think there's any reason to think a reader or implementer would infer the opposite of this; in the descriptor definition, urls is mentioned as a common property, so clearly its presence doesn't imply anything about distributability.
There was a problem hiding this comment.
@jonboulle How so? There has already been confusion that urls == nondistributable, on both docker PRs and OCI PRs. Making this distinction clear will ensure that urls will be useful without being tied to nondistributable.
There was a problem hiding this comment.
I still don't think it's necessary but in the interests of moving forward I added that line, ptal
|
Added a few lines, otherwise, LGTM. You must have a fine forge to be smithing at such a level. |
|
@jonboulle Ping. |
|
LGTM |
|
@jonboulle Looks like the DCO check failed. |
Attempt to clarify when non-distributable policies should be enforced (only on upload, never on download); as discussed in + fixes opencontainers#475 Signed-off-by: Jonathan Boulle <[email protected]>
Signed-off-by: Jonathan Boulle <[email protected]>
|
le sigh. now with fixed DCO |
|
LGTM |
1 similar comment
|
LGTM |
Attempt to clarify when non-distributable policies should be enforced
(only on upload, never on download); as discussed in + fixes #475
Signed-off-by: Jonathan Boulle [email protected]