xanderbailey commented on code in PR #2677:
URL: https://github.com/apache/iceberg-rust/pull/2677#discussion_r3578148166
##########
crates/iceberg/src/transaction/snapshot.rs:
##########
@@ -485,6 +488,28 @@ impl<'a> SnapshotProducer<'a> {
let writer_next_row_id = manifest_list_writer.next_row_id();
manifest_list_writer.close().await?;
+ let mut updates = Vec::new();
+
+ let encryption_key_id = match encrypted {
+ Some((em, enc)) => {
+ let key_id = em
+ .encrypt_manifest_list_key_metadata(enc.key_metadata())
+ .await?;
+
+ updates.extend(em.with_encryption_keys(|keys| {
+ keys.values()
+ .filter(|k|
self.table.metadata().encryption_key(k.key_id()).is_none())
+ .map(|k| TableUpdate::AddEncryptionKey {
+ encryption_key: k.clone(),
+ })
+ .collect::<Vec<_>>()
+ }));
+
+ Some(key_id)
+ }
+ None => None,
+ };
Review Comment:
Consider moving this to a function and unit testing it.
##########
crates/iceberg/src/transaction/snapshot.rs:
##########
@@ -443,28 +443,31 @@ impl<'a> SnapshotProducer<'a> {
let manifest_list_path = self.generate_manifest_list_file_path(0);
let next_seq_num = self.table.metadata().next_sequence_number();
let first_row_id = self.table.metadata().next_row_id();
- let writer = self
+
+ let raw_output = self
.table
.file_io()
- .new_output(manifest_list_path.clone())?
- .writer()
- .await?;
+ .new_output(manifest_list_path.clone())?;
+
+ let (writer, encrypted) = if let Some(em) =
self.table.encryption_manager() {
+ let enc = em.encrypt(raw_output);
+ (enc.writer().await?, Some((em, enc)))
Review Comment:
Rather than returning a tuple of tuples, can we just return the key metadata
and the writer?
##########
crates/iceberg/src/transaction/snapshot.rs:
##########
@@ -443,28 +443,31 @@ impl<'a> SnapshotProducer<'a> {
let manifest_list_path = self.generate_manifest_list_file_path(0);
let next_seq_num = self.table.metadata().next_sequence_number();
let first_row_id = self.table.metadata().next_row_id();
- let writer = self
+
+ let raw_output = self
.table
.file_io()
- .new_output(manifest_list_path.clone())?
- .writer()
- .await?;
+ .new_output(manifest_list_path.clone())?;
+
+ let (writer, encrypted) = if let Some(em) =
self.table.encryption_manager() {
+ let enc = em.encrypt(raw_output);
Review Comment:
```suggestion
let encrypted_output = em.encrypt(raw_output);
```
--
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]
