tombentley commented on a change in pull request #11773:
URL: https://github.com/apache/kafka/pull/11773#discussion_r819434434



##########
File path: 
connect/api/src/main/java/org/apache/kafka/connect/source/SourceConnector.java
##########
@@ -28,4 +30,46 @@
     protected SourceConnectorContext context() {
         return (SourceConnectorContext) context;
     }
+
+    /**
+     * Signals whether the connector supports exactly-once delivery guarantees 
with a proposed configuration.
+     * Developers can assume that worker-level exactly-once support is enabled 
when this method is invoked.
+     *
+     * <p>For backwards compatibility, the default implementation will return 
{@code null}, but connector developers are
+     * strongly encouraged to override this method to return a non-null value 
such as
+     * {@link ExactlyOnceSupport#SUPPORTED SUPPORTED} or {@link 
ExactlyOnceSupport#UNSUPPORTED UNSUPPORTED}.
+     *
+     * <p>Similar to {@link #validate(Map) validate}, this method may be 
called by the runtime before the
+     * {@link #start(Map) start} method is invoked when the connector will be 
run with exactly-once support.

Review comment:
       Ultimately there's a cyclic dependency:
   
   1. We (or at least I) would like for `Connector::exactlyOnceSupport` and 
`Connector::canDefineTransactionBoundaries` to be called with only with valid 
config parameters (implying that `Connector::validate` has been called first).
   2. But we'd like to call `Connector::validate` having first validated the 
`transaction.boundary` and `exactly.once.support`, (which implies calling 
`Connector::exactlyOnceSupport` and 
`Connector::canDefineTransactionBoundaries`).
   
   But in 2. do we really need to call `Connector::exactlyOnceSupport` and 
`Connector::canDefineTransactionBoundaries` though, or it is enough to validate 
`transaction.boundary` and `exactly.once.support` are legal enum values 
(terminating early if they're not), and then call 
`Connector::exactlyOnceSupport` and `Connector::canDefineTransactionBoundaries` 
after `Connector::validate`? We could add any 2nd phase `exactlyOnceSupport` 
and `canDefineTransactionBoundaries` errors to the `ConfigValues` from 
`validate`? That would then establish the pattern of per-config syntax 
validation happening before any inter-config validation (since typically a 
connector overriding `validate` would call `super`'s impl first thus achieving 
ConfigDef validation). I realise the KIP approved these signatures (so it's 
getting a bit late to change them), but I think that would allow 
`Connector::exactlyOnceSupport(Config)` and 
`Connector::canDefineTransactionBoundaries(Config)`, which makes the ordering 
much 
 more explicit purely from the types involved. Or am I asking the impossible 
(`AbstractHerder#validateConnectorConfig` is not the easiest piece of code to 
reason about)?




-- 
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]


Reply via email to