phet commented on code in PR #4071:
URL: https://github.com/apache/gobblin/pull/4071#discussion_r1825270928


##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/iceberg/IcebergTable.java:
##########
@@ -234,8 +234,7 @@ protected void registerIcebergTable(TableMetadata 
srcMetadata, TableMetadata dst
    *
    * @param icebergPartitionFilterPredicate the predicate to filter partitions
    * @return a list of data files that match the partition filter predicate
-   * @throws TableNotFoundException if error occurred while accessing the 
table metadata
-   * @throws RuntimeException if error occurred while reading the manifest file
+   * @throws IOException if error occurred while accessing the table metadata 
or reading the manifest file
    */
   public List<DataFile> getPartitionSpecificDataFiles(Predicate<StructLike> 
icebergPartitionFilterPredicate)
       throws IOException {

Review Comment:
   wouldn't the last line here also throw an NPE for the same reason?
   ```
     public List<DataFile> getPartitionSpecificDataFiles(Predicate<StructLike> 
icebergPartitionFilterPredicate)
         throws IOException {
       TableMetadata tableMetadata = accessTableMetadata();
       Snapshot currentSnapshot = tableMetadata.currentSnapshot();
       long currentSnapshotId = currentSnapshot.snapshotId();
   ```
   
   looking quickly at this class, there are several places that are not 
resilient to the unexpected circumstance of a table having existing and 
metadata, but not actually any snapshots yet.



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