nbali commented on a change in pull request #16909:
URL: https://github.com/apache/beam/pull/16909#discussion_r816368169
##########
File path:
sdks/java/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaIO.java
##########
@@ -1354,17 +1356,29 @@ private boolean
runnerRequiresLegacyRead(PipelineOptions options) {
}
}
- private static class ReadFromKafkaViaUnbounded<K, V>
+ private abstract static class AbstractReadFromKafka<K, V>
extends PTransform<PBegin, PCollection<KafkaRecord<K, V>>> {
Read<K, V> kafkaRead;
Coder<K> keyCoder;
Coder<V> valueCoder;
- ReadFromKafkaViaUnbounded(Read<K, V> kafkaRead, Coder<K> keyCoder,
Coder<V> valueCoder) {
+ AbstractReadFromKafka(
+ Read<K, V> kafkaRead,
+ Coder<K> keyCoder,
+ Coder<V> valueCoder,
+ KafkaIOReadImplementation implementation) {
+ KafkaIOReadImplementationCompatibility.getCompatibility(kafkaRead)
+ .checkSupport(implementation);
this.kafkaRead = kafkaRead;
this.keyCoder = keyCoder;
this.valueCoder = valueCoder;
}
+ }
+
+ static class ReadFromKafkaViaUnbounded<K, V> extends
AbstractReadFromKafka<K, V> {
Review comment:
Same input, same output, same functionality, same fields, same caller.
Even without the newly introduced extra shared functionality
(`Compatibility.checkSupport`) that screams inheritance to me. Obviously I can
replace that if that's the convention here, but strictly theoretically speaking
IMO the previous implementation was/would be harder to grasp and this actually
makes more sense. When you jump around multiple ptransforms and dofns with
barely different names, but (almost) the same fields - like I did during this
PR - type/class hierarchy and shared call hierarchy is a big help... but as I
said, sure I can duplicate the code so they will be independent ptransforms
again.
--
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]
