vsantonastaso commented on PR #10799: URL: https://github.com/apache/seatunnel/pull/10799#issuecomment-4841933659
> Hi @vsantonastaso , > > Thanks again for pushing this forward. I really like the direction here, and keeping this PR focused as a foundation step makes a lot of sense. > > After review this PR, there is one thing I'd still be a bit uneasy about: `DebeziumAdapterFactory#getAdapter()` currently returns the first provider whose `supports()` method matches. > > I know that works fine as long as there is only one matching adapter, and that is probably true today. The part that worries me is the future shape of this SPI. Since this PR is introducing the contract for connector-specific Debezium adapters, I think it would be safer if the selection rule were deterministic from the start. > > If we ever end up with two providers in the same classloader that both claim the same `connector.class`, the current behavior would depend on `ServiceLoader` / classpath ordering. That kind of ambiguity is usually very hard to notice early and very painful to debug later. > > Would you be open to tightening this a bit so the contract becomes: > > * no match -> fail with a clear error > * exactly one match -> use it > * more than one match -> fail fast and list the matched providers > > I don't see this as a stylistic nit. For a foundation PR like this, it feels worth making the boundary explicit now, while it is still small and easy to reason about. How do you think ? Addressed and CI green -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
