EnricoMi commented on code in PR #45462:
URL: https://github.com/apache/arrow/pull/45462#discussion_r2622067506
##########
python/pyarrow/tests/test_dataset_encryption.py:
##########
@@ -82,11 +88,11 @@ def create_decryption_config():
return pe.DecryptionConfiguration(cache_lifetime=300)
-def create_kms_connection_config():
+def create_kms_connection_config(keys):
return pe.KmsConnectionConfig(
custom_kms_conf={
- FOOTER_KEY_NAME: FOOTER_KEY.decode("UTF-8"),
- COL_KEY_NAME: COL_KEY.decode("UTF-8"),
+ key_name: key.decode("UTF-8") if isinstance(key, bytes) else key
Review Comment:
right
##########
python/pyarrow/tests/parquet/encryption.py:
##########
@@ -41,6 +41,8 @@ def wrap_key(self, key_bytes, master_key_identifier):
def unwrap_key(self, wrapped_key, master_key_identifier):
"""Not a secure cipher - just extract the key from
the wrapped key"""
+ if master_key_identifier not in self.master_keys_map:
+ raise ValueError("Unknown master key", master_key_identifier)
Review Comment:
I found
ValueError: ('Unknown master key', 'XYZ')
more expressive than
KeyError: 'XYZ'
##########
cpp/src/arrow/dataset/file_parquet_encryption_test.cc:
##########
@@ -79,14 +83,20 @@ const auto kAllParamValues = ::testing::Values(
EncryptionTestParam{false, true, true}, EncryptionTestParam{true, true,
true});
// Base class to test writing and reading encrypted dataset.
-class DatasetEncryptionTestBase : public
testing::TestWithParam<EncryptionTestParam> {
+template <typename T, typename = typename std::enable_if<
+ std::is_base_of<EncryptionTestParam,
T>::value>::type>
+class DatasetEncryptionTestBase : public testing::TestWithParam<T> {
public:
#ifdef ARROW_VALGRIND
static constexpr int kConcurrentIterations = 4;
#else
static constexpr int kConcurrentIterations = 20;
#endif
+ static const EncryptionTestParam& GetParam() {
Review Comment:
Accessing `GetParam` could not derive return type of
`testing::WithParamInterface<T>::GetParam`:
there are no arguments to ‘GetParam’ that depend on a template
parameter, so a declaration of ‘GetParam’ must be available
Fixed accessing `GetParam` in 473839e597.
##########
cpp/src/arrow/dataset/file_parquet_encryption_test.cc:
##########
@@ -398,9 +411,191 @@ TEST_P(DatasetEncryptionTest, ReadSingleFile) {
INSTANTIATE_TEST_SUITE_P(DatasetEncryptionTest, DatasetEncryptionTest,
kAllParamValues);
+struct NestedFieldsEncryptionTestParam : EncryptionTestParam {
Review Comment:
As in
struct NestedFieldsEncryptionTestParam : public EncryptionTestParam {
IDE says `public` is redundant 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
