Remove feature flags from ConnectorSpecification

[cgardens]

Tell us about the problem you're trying to solve

• ConnectorSpecification (link) contains multiple platform feature flags. This was a pattern from early on in the protocol that we want to move away from. The protocol should vigorously avoid these sorts of feature flags.
• When we do want them, they should be added to the ActorDefinition instead.

Describe the solution you’d like

• Remove supportsIncremental from the ConnectorSpecification -- this field should be deleted.
• Remove supportsNormalization from the ConnectorSpecification -- this can be moved to the ActorDefinition (unless the destinations team gets rid of normalization entire, in which case it could be totally removed).
• Remove supportsDBT from the ConnectorSpecification -- this can be moved to the ActorDefinition.

I am creating this issue, because it is pretty common that people see these flags in the protocol and want to add additional flags there. We don't want to do that! Put it in the actor definition!!! Spread the good word.

@salima-airbyte @evantahler -- for visibility.

[gosusnp]

Trying to clarify what you call feature flag here, looking at the attributes, they sound like feature supported by the connector more than "feature flag" that I understand as transitional, thinking AB testing for example.

Unless I missed something, ActorDefinition could be confusing for feature support. It is part of the API and whether a connector supports incremental, should be part of the connector definition. Taking the example of adding a custom connector, when I add it to my instance of Airbyte, I would then need to check the list of all the features supported by the connector so that it's imported with the right feature set.

We may want to change how we store this information, somehow, ConnectorSpecifications still feels like a good place for this given our current model.

[cgardens]

The Airbyte Protocol describes a series of standard components and all the interactions between them in order to declare an ELT pipeline. All message passing across components is done via serialized JSON messages for inter-process communication.

protocol docs

The protocol should focus on attributes that are vital for these docker containers to communicate with each other, not features of the Airbyte platform. There may be a flag in the future that does make sense to put in the ConnectorSpecification, but I don't know what it is. In addition, all of the flags we currently have are obviously in the wrong place (more on that below). I do not want developers thinking that the protocol is where this sort of flag should go. It should be exceedingly rare (to the point where I cannot think of one). Right now, by default developers still want to put these features here instead of in the ActorDefinition (example). We have to break the team out of that mentality.

In the case of the 3 existing flags, all of them were a mistake from the beginning. This makes them toxic, because they are normalizing a bad pattern:

• supportsIncremental - this was a terrible place to put this piece of information. in each stream in the catalog, it describes whether it supports incremental or not. the concept of a connector supporting incremental doesn't make sense. we only care about that support at the stream-level. we don't use this field anymore (i hope).
• supportsNormalization and supportsDBT - refer to additional docker containers the platform knows how to run. it is not even describing something in the the connector container. this is very clearly a feature of the airbyte app and has nothing to do with out containers communicate with each other.

your example of a user having to too much metadata when creating a source is a good one, but at least in the case of the existing flags, someone who is creating a custom connector would never have to set these fields because they aren't actually part of building a source.

[evantahler]

Double checking that this jives with the ActorDefinition work being done here #19799

[cgardens]

@evantahler yes! good example of us leveraging the ActorDefinition for connector-specific behavior that shouldn't belong in the platform nor protocol.

[gosusnp]

I agree, we should be mindful of not trying to put everything there. supportNormalization and supportDBT should not have been there, but I think this is becoming clearer because the requirements there are shifting.

There are some functionality that impacts how the platform communicates with the connectors, thinking of State type or OAuth for example. Some form of descriptions could help the platform know the expected behavior rather than guessing from the first messages we see.
In a way, I find ActorDefinition very ambiguous because we currently store the Spec there as well.

When the discussions on versioning started, one distinction was protocol versioning versus feature support. I think it makes sense to have room for a feature support exchange in the protocol, however, the solution should not require listing all existing features in the protocol definition. Probably an approach more similar to optional headers in HTTP.

[cgardens]

@gosusnp maybe the distinction we need to make is between platform features and protocol features. If we come up with a feature of the protocol that we want to feature flag, then potentially the spec may be a place to put it. But for features of the Airbyte platform those should not be in the protocol. Does that seem fair / clear?

[gosusnp]

This distinction makes sense to me.

What I want to challenge is that even though they are platform features, they still involve some form of communication between the connectors and the platform. (ie: platform: please tell me what you support, connectors: I can do x, y, z).
Saying that it is entirely independent from the protocol means that we need to look at adding an additional form of communication, that's why I think the protocol is a good place to define a channel for communicating features, but it should try to define the content of this channel.

[evantahler]

Closed by #18239