Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager by DM. When contacting the review manager directly, please put "SE-0536" in the subject line.
What goes into a review?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
More information about the Swift evolution process is available at:
I'm confused by the registry key - since that's never computed by the server, why is it in this Results object? Where is that applicable? Is that only for --json output? In that case, maybe it's clearer if you add this into the section on --json instead
Pagination
For a given query string and constant pagination parameters, result ordering MUST be stable across requests in the same session. A client iterating offset += limit until offset >= total MUST visit every matching package exactly once and never visit the same package twice.
What is a "session" from the standpoint of the server? From the client, it makes sense. But the server has no way to identify that we're in the same "session" as we were before, unless I'm missing something.
Without that I do not understand how a server will ever fulfill this promise.
Have you considered after (being the identity of the last result in the previous page if navigating forward) and before (being the identity of the first result in the "after" page if navigating backwards) pagination rather than offset + limit? It's often much easier to support stable pagination server-side this way.
Another option would be to only rely on the Link header, so the server could decide on its own how to manage this & it could be opaque to the client.
Side note: Has there been an attempt to put together an example registry server implementing this endpoint? Might be a useful exercise.
I would like to suggest slightly improved clarity around when something is expected to be deterministic or allows for fuzzy matching within the scope of part of this API.
In particular, I think the qualifiers scope, author, and pkg should be explicitly deterministic, and not allow fuzzy matching, especially for the examples of filtering. My primary reasoning is that this can lead to unexpected expansions of filters or constraints on the search.
I wouldn't want to say this is a required feature of any implementation, but it should is one that I'd look for and use.
I suspect we might want to consider the same "it's always deterministic/not fuzzy" for when we use name as well.
In short - a fuzzy description, or fuzzy general (unscoped) query, is where it provides the most value, and the expansion effects are what we intentionally want to allow (although not mandate).
I'm very happy to see this enter review. I do have a few comments:
My main concern echos @willft's concern about this statement:
For a given query string and constant pagination parameters, result ordering MUST be stable across requests in the same session. A client iterating offset += limit until offset >= total MUST visit every matching package exactly once and never visit the same package twice.
If this is truly a MUST requirement, this increases the burden on implementation quite significantly and an offset/limit approach might not work as there is chance that the registry state will change between requests. Implementing this as a "MUST" would mean the server would need to keep track of search results at a specific point in time and maybe provide a cursor parameter to allow a client to get back to that same search data.
The specification also mentions "the same session", which is potentially a reference to some kind of cursor-able solution, but this may need some clarification if it is.
This problem also affects the swift package-registry search command, which allows cross-registry search. Even with the cursor suggestion above, a multi-registry search would be a complex problem as we would need to return all cursor references from the command to let them resume the search after the limit. This also becomes a little tricky in the following circumstance:
Registry One - Has 5 matching results for a query
Registry Two - Has 30 matching results for the same query
Command is run with a limit of 20
The command runs and Registry One is queried for 10 results and Registry Two for 10, making a maximum of the limit of 20 results requested by the user.
The results are that Registry One returns 5 results and Registry Two returns 10, making 15 total, even though there are more than 20 results available.
This could be mitigated with each registry being queried for the limit number of results, but this increases complexity quite a bit as you can easily get into a situation where, if both registries have enough results to fulfil their half of the results, the cursors will have overextended and results will be missed.
The easiest solution to the mutation issue is to change this to be a SHOULD rather than a MUST. However, that still leaves cross-registry search in a tricky position. Even if we accept potentially unstable results, the CLI still has to calculate pagination offsets across multiple independent registries without any shared state.
I have one other, more minor point, too:
We might want to be careful with allowing pkg search for a full purl, or tighten up what happens when a search is performed for a version that is not current or does not exist. For example, searching for example/package@1.1.1 when example/package has versions 1.0.0 and 2.0.0. It's probably worth thinking about whether this goes past package discovery and into package/version lookup. With both scope and name searches, we may have everything covered without needing pkg search.
@willft Thanks for taking a look, I'll try to address each comment.
I'm confused by the registry key - since that's never computed by the server, why is it in this Results object? Where is that applicable? Is that only for --json output? In that case, maybe it's clearer if you add this into the section on --json instead
SwiftPM can support multiple registries, configured in the registries.json. You instance you can configure dependencies for the initech scope to come from one registry, and all others to come from a different registry. The registry key is amended to the search results by SwiftPM so callers can more easily tell which registry the search result came from. It's actually ambiguous in the proposal whether the registry key appears in non-json results; I'm open to either but leaning towards including in both json/non-json searches it if the search spans multiple registries.
What is a "session" from the standpoint of the server? From the client, it makes sense. But the server has no way to identify that we're in the same "session" as we were before, unless I'm missing something.
This is probably a poor choice of words, since there isn't an explicit piece of state representing a session. I modelled pagination after the existing approach used when listing releases (GET /{scope}/{name}), and I think we should try to be consistent here.
I do think we can strengthen the language here to make it more clear. Hopefully this also addresses @daveverwer's concerns as well:
For a given query and identical underlying data, a registry MUST return results in a deterministic order, independent of the offset and limit requested, so that successive pages partition the result set without gaps or overlaps. When relevance scores are equal, registries MUST apply a stable tiebreaker (for example, ordering by package identity).
Registries are not required to snapshot results across requests. Because a search index changes over time, a client paginating with offset += limit MAY observe packages shift between pages, appear, or disappear if the underlying data changes during iteration. Clients SHOULD tolerate the same identity appearing on more than one page.
Finally,
Side note: Has there been an attempt to put together an example registry server implementing this endpoint? Might be a useful exercise.
I implemented a reference implementation of a swift registry a few weeks ago, and then in the draft PR to add registry search support to SwiftPM I updated this reference implementation to support the /search endpoint:
I think this is a worthwhile improvement. I'd like name: to remain fuzzy. @daveverwer pointed out to me that when you search for markdown on Swift Package Index you'd get an exact match, but the most popular markdown package in Swift is swift-markdown, which means we'd omit the most popular result if we did exact matching on name.
How does this amendment sound:
Fuzzy matching applies only to free-text terms (bare literals), the name: and the description: qualifier. The scope:, author:, and pkg: qualifiers MUST be matched deterministically; a registry MUST NOT apply fuzzy matching to them. These qualifiers remain case-insensitive substring matches as described above.