Idea that came up during the framework SIG (👀 @benjdlambert):

First a bit of context, which is that the entire need for this is this piece of code here:

We need to branch based on whether it's a search URL or not. But what if we didn't need to do that? Rather than having the need to differentiate between search and direct file URLs, could we make the search method essentially short-circuit in the case of a direct URL? We'd still keep the existing API, but in the various implementations of the search methods we would check whether it's a search URL or not using logic that is specific to each provider. If it's not a search URL, we simply defer to the readUrl method for that implementation and return that single file as the search result. With this approach we can still do proper etag handling since that's supported by the search method, and I think really the only remaining question about the implementation would be how we'd handle 404s? Should that result in an error or an empty search result?

I do think that approach is pretty neat, and not really a breaking change in the API itself as it's arguably the correct behavior already. The tricky bit is gonna be the UrlReaderProcessor though. It's a breaking change to switch over to always calling the search method, so I think we may need to have a flaaaag

Thanks for the feedback during the SIG and here!

I've pushed on this PR an attempt with the GithubUrlReader. I'll continue on other readers but I wanted to start small!

We might need to do some change in the return type of the search method: the readUrl returns an optional etag whereas for the search the etag is mandatory. For the GithubUrlReader this requires to get the repo datails first before doing the short-circuit to readUrl.

how we'd handle 404s? Should that result in an error or an empty search result?

I've moved the handling of the NotFoundError error in the search method, to return an empty search result in case a direct URL returns a 404.
The current implementation returns an empty list of files if the glob pattern doesn't match anything, so for me it makes sense to return an empty list if a direct URL returns a 404.
In the UrlReaderProcessor, if the list is empty and the location isn't optional, then it emits an error. Otherwise it's ignored.
WDYT?

The tricky bit is gonna be the UrlReaderProcessor though. It's a breaking change to switch over to always calling the search method, so I think we may need to have a flaaaag

How would you implement this flag? Checking for an environment variable that will be removed eventually in a future release?

How would you implement this flag? Checking for an environment variable that will be removed eventually in a future release?

Let's see if we feel we need it first. It could be a config option, or a separate module. Ideally we're able to ship without that, but I'm not too optimistic about being able to find a solution for that tbh.

Alright, let me know which way you want to go!

Let's move this new behavior behind a config flag so that people can opt-in, and then I think this is good to go 👍

Let's also log a deprecation warning if the flag is not set to encourage people to move over

Here's a similar simple lil' flag:

It needs a config.d.ts entry too like this one:

To get access to config you'll need to pass it in to the constructor of UrlReaderProcessor, it'll need to be optional in case someone is wiring it up manually, but we can log if config isn't provided too in a similar way. Here's another processor that uses config, but skip the fromConfig method, not worth it at this point:

Done!