nbali commented on PR #16888: URL: https://github.com/apache/beam/pull/16888#issuecomment-1103257633
Actually now that https://github.com/apache/beam/pull/16909 has been finally merged, I think we don't even need this anymore. The override replaces `ReadFromKafkaViaSDF` with `ReadFromKafkaViaUnbounded` by calling the constructor of `ReadFromKafkaViaUnbounded`. https://github.com/apache/beam/blob/c6972f4b8d9a03f4639395f60f18e740dce91398/sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaIO.java#L1345-L1371 The constructor of `ReadFromKafkaViaUnbounded` https://github.com/apache/beam/blob/c6972f4b8d9a03f4639395f60f18e740dce91398/sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaIO.java#L1392-L1395 will call `AbstractReadFromKafka` https://github.com/apache/beam/blob/c6972f4b8d9a03f4639395f60f18e740dce91398/sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaIO.java#L1373-L1385 which contains `KafkaIOReadImplementationCompatibility.getCompatibility(kafkaRead).checkSupport(implementation);` https://github.com/apache/beam/blob/c6972f4b8d9a03f4639395f60f18e740dce91398/sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaIOReadImplementationCompatibility.java#L232-L240 which should fail if we try to initiate the DoFn with a property that is not supported. So I think we either get the legacy read without a feature loss, or we fail during the graph building if we would have a feature loss. Is that acceptable then? @kennknowles @chamikaramj -- 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: github-unsubscr...@beam.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
