Blazer-007 commented on code in PR #4142:
URL: https://github.com/apache/gobblin/pull/4142#discussion_r2459857603
##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/iceberg/IcebergPartitionDatasetFinder.java:
##########
@@ -53,6 +53,8 @@ protected IcebergDataset createSpecificDataset(IcebergTable
srcIcebergTable, Ice
boolean validateStrictPartitionEquality =
Boolean.parseBoolean(properties.getProperty(ICEBERG_DATASET_PARTITION_VALIDATE_STRICT_EQUALITY,
DEFAULT_ICEBERG_DATASET_PARTITION_VALIDATE_STRICT_EQUALITY));
+
destIcebergTable.updateSchema(srcIcebergTable.accessTableMetadata().schema(),
true);
+
Review Comment:
nit: let's add a comment describing that this only verifies the schema
compatability and not update the schema as someone reading the code may
interpret wrong if not went completely into `updateSchema` function
##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/iceberg/IcebergOverwritePartitionsStep.java:
##########
@@ -146,17 +165,30 @@ private Retryer<Void> createOverwritePartitionsRetryer() {
?
config.getConfig(IcebergOverwritePartitionsStep.OVERWRITE_PARTITIONS_RETRYER_CONFIG_PREFIX)
: ConfigFactory.empty();
+ return getRetryer(retryerOverridesConfig);
+ }
+
+ private Retryer<Void> createSchemaUpdateRetryer() {
+ Config config = ConfigFactory.parseProperties(this.properties);
+ Config retryerOverridesConfig =
config.hasPath(IcebergOverwritePartitionsStep.SCHEMA_UPDATE_RETRYER_CONFIG_PREFIX)
+ ?
config.getConfig(IcebergOverwritePartitionsStep.SCHEMA_UPDATE_RETRYER_CONFIG_PREFIX)
Review Comment:
nit: can this be combined with above function by passing prefix as function
param ?
##########
gobblin-data-management/src/test/java/org/apache/gobblin/data/management/copy/iceberg/IcebergTableTest.java:
##########
@@ -333,8 +498,9 @@ public void testOverwritePartition() throws IOException {
addPartitionDataFiles(table,
createDataFiles(paths.stream().collect(Collectors.toMap(Function.identity(), v
-> partition1Data))));
+ TableOperations tableOps = catalog.newTableOps(tableId);
IcebergTable icebergTable = new IcebergTable(tableId,
- catalog.newTableOps(tableId),
+ tableOps,
Review Comment:
just curious, is this change required ?
--
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]
