RussellSpitzer commented on code in PR #17545:
URL: https://github.com/apache/iceberg/pull/17545#discussion_r4049244594
##########
api/src/main/java/org/apache/iceberg/encryption/EncryptingFileIO.java:
##########
@@ -140,6 +145,16 @@ public InputFile newInputFile(ManifestListFile
manifestList) {
}
}
+ @Override
+ public InputFile newInputFile(String location, String keyId) {
+ if (keyId != null) {
Review Comment:
I think overall we are still stuck a bit too much mimicking the logic that
was here before for ManifestLists without thinking about how we would add this
API by first principals.
We are essentially adding a new API that a FileIO user can call if they have
key metadata. We know they also will have a path and may have a length. So it
probably makes sense to build this up similar to the other methods in FileIO.
Right now we have
```java
/** Get a {@link InputFile} instance to read bytes from the file at the
given path. */
InputFile newInputFile(String path);
/**
* Get a {@link InputFile} instance to read bytes from the file at the
given path, with a known
* file length.
*/
default InputFile newInputFile(String path, long length) {
return newInputFile(path);
}
```
So we probably also need
```java
/**
* Get a {@link InputFile} instance to read bytes from the file at the
given path, with a known
* file length.
*/
default InputFile newInputFile(String path, long length, String
keyMetadata) {
throws UnsupportedOperationException(can't use without encrypting file
io)
}
```
Then the question would be if you are in EncryptingFileIO should the
implementation fail if keyMetadata is null? Because the library user has
expressly used a method which implies keyMetadata is important, so is it OK to
just fall back to non-ecryption behavior? I don't feel comfortable with that
but others may have a different opinion.
--
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]
