danielcweeks commented on code in PR #9641:
URL: https://github.com/apache/iceberg/pull/9641#discussion_r1501867687


##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/data/IcebergWriter.java:
##########
@@ -52,20 +51,22 @@ public IcebergWriter(Table table, String tableName, 
IcebergSinkConfig config) {
 
   private void initNewWriter() {
     this.writer = Utilities.createTableWriter(table, tableName, config);
-    // FIXME: update this when the record converter is added
-    //  this.recordConverter = new RecordConverter(table, config);
+    this.recordConverter = new RecordConverter(table, config);

Review Comment:
   Should we be doing some sort of validation on the config settings here or in 
the constructor?  What does it mean to configure a cdc field, but not have 
upsert mode enabled?  Alternatively what happens if you have upsert enabled but 
no cdc field?
   
   It just feels like we should do some validation that users are setting 
correlated fields.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to