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 tests 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. I think it's also reasonable to update this test
because this test is touching the writer path.
--
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]
