rdblue commented on code in PR #7770:
URL: https://github.com/apache/iceberg/pull/7770#discussion_r1676566153
##########
core/src/main/java/org/apache/iceberg/SnapshotProducer.java:
##########
@@ -257,10 +273,48 @@ public Snapshot apply() {
.run(index -> manifestFiles[index] =
manifestsWithMetadata.get(manifests.get(index)));
writer.addAll(Arrays.asList(manifestFiles));
+
+ if (manifestListKeyMetadata != null) {
+ writer.close();
+ manifestListSize = writer.length();
Review Comment:
For manifests, we use a pattern where the writer produces the `ManifestFile`
metadata after closing the writer to delegate more responsibility to the writer
and make it easy to produce the metadata. I'm wondering if that's a good idea
here as well, since the writer will have the `EncryptedOutputFile`.
The main difference for the pattern is that the key metadata from the
`EncryptedOutputFile` is unencrypted and needs to be encrypted using the key
encryption key. I don't think I have a strong preference whether this is
delegated to the writer (`toManifestListFile()`) or just done in a method like
`ManifestListFile.create(EncryptionManager em, String wrappedKey, ByteBuffer
keyMetadataBuffer, String location)`. Either way, I don't think that it should
be done here.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
