phet commented on code in PR #3643:
URL: https://github.com/apache/gobblin/pull/3643#discussion_r1122516199
##########
gobblin-data-management/src/test/java/org/apache/gobblin/data/management/copy/iceberg/IcebergDatasetTest.java:
##########
@@ -187,7 +188,7 @@ public void
testGetFilePathsDoesNotSwallowDestFileSystemException() throws IOExc
MockFileSystemBuilder sourceFsBuilder = new
MockFileSystemBuilder(SRC_FS_URI);
FileSystem sourceFs = sourceFsBuilder.build();
- IcebergDataset icebergDataset = new IcebergDataset(testDbName,
testTblName, icebergTable, new Properties(), sourceFs);
+ IcebergDataset icebergDataset = new IcebergDataset(testDbName,
testTblName, icebergTable, SRC_CATALOG_URI, new Properties(), sourceFs);
Review Comment:
in the suggestion above, this additional new param goes away
##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/iceberg/IcebergDatasetFinder.java:
##########
@@ -92,6 +103,18 @@ public Iterator<IcebergDataset> getDatasetsIterator()
throws IOException {
protected IcebergDataset createIcebergDataset(String dbName, String tblName,
IcebergCatalog icebergCatalog, Properties properties, FileSystem fs) {
IcebergTable icebergTable = icebergCatalog.openTable(dbName, tblName);
- return new IcebergDataset(dbName, tblName, icebergTable, properties, fs);
+ return new IcebergDataset(dbName, tblName, icebergTable,
icebergCatalog.getCatalogUri(), properties, fs);
Review Comment:
seems worthwhile to keep around the `IcebergCatalog.getCatalogUri()` method,
but rather than calling here, why not have that catalog pass its URI to the
`IcebergTable`, as the former is the very one to create the latter?
subsequently `IcebergDataset` will call `IcebergTable.getDatasetDescriptor`,
w/o needing to pass in a URI arg.
--
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]
