fallintoplace commented on code in PR #1673:
URL: https://github.com/apache/iceberg-go/pull/1673#discussion_r3731450437


##########
table/snapshot_producers.go:
##########
@@ -1328,6 +1364,12 @@ func (sp *snapshotProducer) 
commitManifests(newManifests, addedContent []iceberg
                                return nil, fmt.Errorf("rebuild manifest list: 
write: %w", wErr)
                        }
                }
+               if closeErr := closeManifestListOutput(fio, manifestListPath, 
out, writer, false); closeErr != nil {
+                       closed = true
+
+                       return nil, fmt.Errorf("rebuild manifest list: close: 
%w", closeErr)
+               }
+               closed = true

Review Comment:
   Thanks, you were right that this could be simpler. I checked the current 
Java SnapshotProducer implementation and the relevant writer and IO contracts. 
Java closes the manifest-list writer before constructing the snapshot. In Go, 
the manifest-list writer closes the Avro writer, while the separate FileWriter 
still needs to be closed to finalize the blob.
   



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

Reply via email to