amogh-jahagirdar commented on code in PR #17960:
URL: https://github.com/apache/iceberg/pull/17960#discussion_r3938498682


##########
kafka-connect/kafka-connect/src/test/java/org/apache/iceberg/connect/data/WriterTestBase.java:
##########
@@ -64,14 +68,20 @@ public class WriterTestBase {
   public void before() {
     fileIO = new InMemoryFileIO();
 
-    table = mock(Table.class);
+    table = mock(Table.class, 
withSettings().extraInterfaces(HasTableOperations.class));

Review Comment:
   Flagging why I'm touching the kafka-connect module here:
   
   1. MetricsConfig.forTable(table) now calls TableUtil.formatVersion(table), 
which only resolves for a Table that's a SerializableTable, HasTableOperations, 
or BaseMetadataTable. Anything else throws.
   2. I think that's a reasonable requirement. MetricsConfig tells writers what 
to collect, and if you're writing, you're going to commit, which needs 
TableOperations anyway. I didn't add a Preconditions check for it since the 
formatVersion exception already covers it if this assumption ever breaks but we 
can add one. I can't imagine a MetricsConfig being more generalizable than this 
use case? 
   
   WriterTestBase mocked a Table with no operations backing it and never had to 
satisfy this before, so I updated it to implement HasTableOperations with a 
stubbed format version.



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