resource paths in source-http-ingest

[psFried]

This started out as just me collecting my thoughts related to some source-http-ingest changes to bring it in line with the new resource_path merging. But it was tricky to see how to do this without breaking things, and it ended up leading to what I think is a pretty reasonable conclusion about resource_paths and discovery merging. So I apologize if this is difficult to follow. Please bear with me.

The main goal here was to avoid introducing breaking changes in how connectors handle resource_paths. Overall, it was very easy to avoid any breaking changes, but source-http-ingest proved to be kind of an exeption.

The current resource config for source-http-ingest is defined like so:

pub struct ResourceConfig {
    /// The URL path to use for adding documents to this binding. Defaults to the name of the collection.
    #[serde(default)]
    pub path: Option<String>,
    /// Set the /_meta/webhookId from the given HTTP header in each request.
    ///
    /// If not set, then a random id will be generated automatically. If set, then each request will be required
    /// to have the header, and the header value will be used as the value of `/_meta/webhookId`.
    #[serde(default)]
    pub id_from_header: Option<String>,
}

The path field corresponds to the URL path that the server will accept for requests to add data. The path is certainly the most reasonable thing to use for the resource_path, but the first-order problem is that it's optional, and not set at all by default. And if it's not specified, then it defaults to the full name of the flow collection that's the target of the binding. So for example, a typical binding would look like:

- resource: { path: null, id_from_header: null }
  target: acmeCo/foo/webhook-data

and the resulting resource_path would be acmeCo/foo/webhook-data.

This means that discover cannot emit an equivalent resource_path, and thus the next discover operation will remove any existing bindings. Luckily, of the 19 live source-http-ingest captures, all 19 of them have auto-discover disabled, and also have completely default bindings. So, the next discover (which will have been triggered manually via the UI) will remove their existing bindigs. But it seems like any newly discovered binding would be identical to the ones that are already there.

Obviously the way we model the resource config is not ideal here. Minimally, we must be able to return an identical resource_path from both the discover and validate RPCs, which is impossible given the current behavior of path. We do at least have some ability to change how we determine the resource_path, since there's no live captures with autoDiscover enabled. But we also need to take care not to change the URL paths that are used for existing captures.

Ideally, we would allow the 19 existing captures to keep using their same URLs, and also allow them to enable autoDiscover.

Using the path as the resource_path seems like the most reasonable mapping. In other words, the "resource" that the source-http-ingest connector can be said to be the namespace of URL paths.

Here's what I think we'd need to do in order to make that work and retain seamless compatibility for all existing tasks:

• Update the connector to always set a non-null path as part of the resource returned by discover
  • The default path that's used in returned from discovers would just be webhook-data, not a full collection name
• short term: Continue to allow the path to be null in the resource and retain the current validate behavior that falls back to using the collection as the resource_path.
• long term: We can update validate to require a non-null path once all existing captures have been updated
• Update the connector to allow HTTP requests to either the path or the collection name

That last point is a little gross, but necessary to prevent breaking existing captures if they re-discover and publish.

Left turn?

This is where we intersect with another recent discussion about resource_paths and the merge behavior related to "custom" bindings. Custom bindings here are defined as any bindings that don't appear in the discover output, as determined by their resource path. For source-http-ingest, this effectively means that changing the URL path would make this a "custom" binding, which as things are currently will result in the binding being removed by subsequent discover operations.

So what if we were less fussy about the resource_path modeling the namespace of URL paths, and instead just said is any old string identifier? The update to source-http-ingest could then be to simply hard-code the resource_path to some constant, and let the path continue to work as it does tooday and all existing captures are fully compatible.

But this raises the question of what happens if you have two capture bindings that both have the same resource_path. This could technically happen today, but it would be guaranteed to occur if we hard-coded every resource_path to the same constant in source-http-ingest.
For materializations, this would cause a validation error because we explicitly try to prevent users from materializing two collections into the same database table.

But I would propose that we take a slightly different view of resource_path, at least for captures. Why not allow multiple capture bindings to have the same resource_path? To allow it only requires a tiny, but subtle change to the way we implement the discover merge. It would provide a simple solution for how to handle resource paths in source-http-ingest, and would also provide a workable solution for supporting "custom" bindings with discovery in other connectors.

The discover merge logic would just need to pull in every live binding that matches each discovered binding, instead of just the first matching binding as it does today. In other words, today we expect and assume that live bindings match 1-1 with discovered bindings, and we choose one arbitrarily if that turns out not to be the case. But if we just allow the merge logic to pull in every matching live binding, then we can gain some useful flexibility. I'm inclined at this point to move forward with allowing multiple capture bindings to have the same resource_path, but would like to pause and see if anyone else has thoughts on the matter.

[jgraettinger]

What about adding a new and (initially) optional property stream to the config?

• If stream is omitted, it's default value is webhook-data.
• The resource path is []{stream}
• The behavior of path remains unchanged.
• Discovery returns a single binding with stream: webhook-data and recommended name webhook-data.
• We have the option of making stream not optional, by updating existing capture specs accordingly.

This doesn't smell at all to me ☝️ . If you want to have more than one binding, you need to name them. At the same time, the current behavior of path is nice and I'd hate to lose it (plus it would break existing captures).

Edit to add: what this is buying, is that users can both use auto-discovery and also customize the path of their single discovered binding if they like.

[jgraettinger]

Why not allow multiple capture bindings to have the same resource_path?

I'll flip the question around: why should captures allow multiple bindings to have the same resource path?

A resource path reflects a unique resource in the captured endpoint. That means we're choosing to capture the same resource multiple times through multiple bindings. Why?

This question seems to imply that whole-cloth custom bindings would be returning the same resource path. This doesn't make sense to me, because they still reflect some named view or other interesting resource of the endpoint. Resource paths are useful task metadata that are potentially deserving of being surfaced via UI. To that end shouldn't we want to push the user towards annotating custom bindings with useful resource paths?

Another concern is that we're intending to use resourcePath + backfillVersion as a universal platform key for binding-level connector states.

[psFried]

Binding states being keyed off of resourcePath + bakfillVersion seems like a good enough reason.
Your suggestion of stream was exactly the line of thinking that led me here. If stream is just an arbitrary string, then why would we require that it's unique? The answer is essentially that, because resourcePath + backfillVersion is used to key states, the system is fundamentally incapable of capturing the same resource (as identified by resourcePath) twice. That seems like a fine limitation, honestly.

And this also answers the question of how we should handle the case of duplicate resource paths. Like with materializations, it must result in a validation error.

I think if we were starting fresh, I'd really want to combine stream and path into a single required property. But given where we're at now, I think adding stream seems pretty reasonable. I'll go with that 👍