yifan-c commented on code in PR #175:
URL: 
https://github.com/apache/cassandra-analytics/pull/175#discussion_r2881614555


##########
cassandra-analytics-cdc-codec/src/main/java/org/apache/cassandra/cdc/kafka/KafkaPublisher.java:
##########
@@ -62,7 +63,8 @@ public class KafkaPublisher implements AutoCloseable
     ThreadLocal.withInitial(HashMap::new);
     protected final KafkaStats kafkaStats;
 
-    public KafkaPublisher(TopicSupplier topicSupplier,
+    public KafkaPublisher(String version,

Review Comment:
   Thanks for calling it out. 
   Thinking from API design perspective, how about passing `CassandraVersion`, 
instead of String version value? It, then, ensures the version is always valid. 
Sidecar anyways has access to `CassandraVersion`. It would be clear for Sidecar 
to create the version object. 



##########
cassandra-analytics-common/src/main/java/org/apache/cassandra/cdc/msg/RangeTombstoneBuilder.java:
##########
@@ -28,37 +28,17 @@
  *
  * @param <T>
  */
-public abstract class RangeTombstoneBuilder<T>
+public interface RangeTombstoneBuilder<T>

Review Comment:
   maybe add the note in code so it stays as interface. 



##########
cassandra-analytics-cdc/src/main/java/org/apache/cassandra/bridge/CdcBridgeFactory.java:
##########
@@ -150,12 +154,35 @@ private static VersionSpecificBridge create(@NotNull 
String label)
             {
             }
 
-            return new VersionSpecificBridge(bridgeInstance, 
cdcBridgeInstance, cqlToAvroSchemaConverter);
+            return new VersionSpecificBridge(bridgeInstance, 
cdcBridgeInstance, cqlToAvroSchemaConverter, loader);
         }
         catch (ClassNotFoundException | NoSuchMethodException | 
InstantiationException
                | IllegalAccessException | InvocationTargetException exception)
         {
             throw new RuntimeException("Failed to create Cassandra bridge for 
label " + label, exception);
         }
     }
+
+    public static <T> T executeActionOnBridgeClassLoader(@NotNull 
CassandraVersion version, Throwing.Function<ClassLoader, T> action)

Review Comment:
   nit: add `@VisibleForTesting`



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to