Skip to content
This repository was archived by the owner on May 15, 2024. It is now read-only.

Update filtering response behavior #81

Closed
wants to merge 1 commit into from

Conversation

michaelb990
Copy link
Contributor

This strengthens the recommendation for filtering
from MAY => SHOULD. Additionally, it requires implementations
which do not implement filtering to return an error when they
are unable to fulfill a client request. Clients that work with
implementations which do not support filtering who receive this
error should retry the request without a filter query parameter.

Would love feedback about the following:

  • Is SHOULD the right level here? I still don't like the inconsistent behavior this could result in, and I would love feedback on whether it should go to MUST. I don't see a reason to let registries choose to implement filtering or not and make clients handle the fallback cases.

This strengthens the recommendation for filtering
from MAY => SHOULD. Additionally, it requires implementations
which do not implement filtering to return an error when they
are unable to fulfill a client request. Clients that work with
implementations which do not support filtering who receive this
error should retry the request without a filter query parameter.

Signed-off-by: Michael Brown <[email protected]>
@michaelb990 michaelb990 requested a review from a team as a code owner December 17, 2021 18:51
@SteveLasker
Copy link
Contributor

Adding xref to #80

@SteveLasker SteveLasker added this to the 1.0.0-draft.2 milestone Jan 6, 2022
Copy link
Contributor

@SteveLasker SteveLasker left a comment

Choose a reason for hiding this comment

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

LGTM

@justincormack
Copy link

An implementation might not support filtering as it is simply a static website - in which case it would not be possible to return a 400. So it would be nicer to just let a client that cannot filter return all results, and expect the client to work this out. We have not previously ruled out static websites for hosting registries.

@SteveLasker
Copy link
Contributor

static sites: considering the type of indexing required for referrers, and all the other aspects we're adding, is a static site realistic?

@michaelb990
Copy link
Contributor Author

That's an interesting point @justincormack. I think it applies to some of what we're talking about in #82 as well. In general, I agree we should strive to create APIs that are as simple as possible. I wasn't aware that hosting a registry as a static website was a specific tenant of the OCI API spec, but it's definitely a useful lens to use when thinking about changes. Are there any example implementations that you're aware of that use a static site today?

In that case, it seems like it might be best to close this PR in favor of the way things were originally.

@SteveLasker
Copy link
Contributor

Is a static site really practical? Particularly for this usecase? If we think its more than an exercise of the imagination, yes. But, I don't see the practicality of implementing a static site that indexes content. The value of the error handling Michael provided seems more valuable than the "risk" if there is any.
A real-world example would possibly influence that

@justincormack
Copy link

justincormack commented Jan 12, 2022

Why is it not possible? You just statically provide the output of all the references, its not much different from making a static blog that has lookups by keywords. While its not an explicit aim, I find any http based tool that cant be implemented as a static site to potentially be not well designed.

Here is a static registry example https://github.com/NicolasT/static-container-registry

In this place, a trivial protocol change makes it work, not requiring the server to 400 on search, but to allow fallback to unfiltered, which reduces round trips too.

@SteveLasker
Copy link
Contributor

@michaelb990, @sajayantony, what do you think?

@michaelb990
Copy link
Contributor Author

I think I agree with @justincormack that we don't need this to be required. For small registries, the value isn't very high and keeping it optional will (hopefully) encourage clients to implement filtering as well.

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

Successfully merging this pull request may close these issues.

3 participants