[GitHub] [arrow] adamreeve commented on a diff in pull request #34181: GH-25986: [C++] Enable external material and rotation for encryption keys

["adamreeve (via GitHub)" <gi...@apache.org>]

adamreeve commented on code in PR #34181:
URL: https://github.com/apache/arrow/pull/34181#discussion_r1111311533


##########
cpp/src/parquet/encryption/file_key_unwrapper.h:
##########
@@ -44,14 +43,21 @@ class PARQUET_EXPORT FileKeyUnwrapper : public DecryptionKeyRetriever {
   /// key_toolkit and kms_connection_config is to get KmsClient from cache or create
   /// KmsClient if it's not in the cache yet. cache_entry_lifetime_seconds is life time of
   /// KmsClient in the cache.
+  /// If the file uses external key material then either the Parquet file path and file
+  /// system must be specified, or a key material store provided.
   FileKeyUnwrapper(KeyToolkit* key_toolkit,
                    const KmsConnectionConfig& kms_connection_config,
-                   double cache_lifetime_seconds);
+                   double cache_lifetime_seconds, const std::string& file_path,
+                   const std::shared_ptr<::arrow::fs::FileSystem>& file_system = NULLPTR,
+                   std::shared_ptr<FileKeyMaterialStore> key_material_store = NULLPTR);

Review Comment:
   It might make sense for `CryptoFactory::GetFileEncryptionProperties` and `CryptoFactory::GetFileDecryptionProperties` to have separate overloads as they seem closer to the public API that most people will interact with? I'm not so sure about this constructor, it's in the public encryption namespace but does seem closer to the internals, and is only called from `CryptoFactory::GetFileDecryptionProperties` within the arrow source, where we pass all parameters.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org