chamikaramj commented on code in PR #39973:
URL: https://github.com/apache/beam/pull/39973#discussion_r3926576921


##########
sdks/java/io/iceberg/src/main/java/org/apache/beam/sdk/io/iceberg/SerializableDeleteFile.java:
##########
@@ -62,7 +66,11 @@ public static SerializableDeleteFile.Builder builder() {
   @SchemaFieldNumber("4")
   public abstract long getFileSizeInBytes();
 
+  /**
+   * @deprecated Use {@link #getJsonPartition()} instead.

Review Comment:
   Nit: probably also mention this deprecation in the CL description.



##########
sdks/java/io/iceberg/src/main/java/org/apache/beam/sdk/io/iceberg/SerializableDeleteFile.java:
##########
@@ -228,7 +282,21 @@ public DeleteFile createDeleteFile(
             .withMetrics(metrics)
             .withSplitOffsets(getSplitOffsets())
             .withEncryptionKeyMetadata(getKeyMetadata())
-            .withPartitionPath(getPartitionPath());
+            .withReferencedDataFile(getReferencedDataFile());
+
+    @Nullable String jsonPartition = getJsonPartition();
+    if (jsonPartition != null) {
+      try {
+        deleteFileBuilder = 
deleteFileBuilder.withPartition(partition(partitionSpec));
+      } catch (RuntimeException e) {

Review Comment:
   Nit: seems like this catch might be too broad ? Should we either catch a 
Exception that is less broad or rethrow  ?



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

Reply via email to