Skip to content

Creating the GetExternalFileEncryptionProperties us part of task #26#39

Merged
cristekdatum merged 4 commits intodev_phase2from
26
Aug 11, 2025
Merged

Creating the GetExternalFileEncryptionProperties us part of task #26#39
cristekdatum merged 4 commits intodev_phase2from
26

Conversation

@cristekdatum
Copy link
Copy Markdown
Collaborator

@cristekdatum cristekdatum commented Aug 9, 2025

As part of #11 this PR is adding the python bindings to use the FileEncryptor

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 9, 2025

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

See also:

@cristekdatum cristekdatum marked this pull request as ready for review August 10, 2025 00:53
Comment thread cpp/src/parquet/encryption/crypto_factory.h Outdated
Comment thread python/pyarrow/_parquet_encryption.pxd Outdated
Comment thread python/pyarrow/_parquet_encryption.pyx Outdated
Comment thread python/pyarrow/includes/libparquet_encryption.pxd Outdated
Comment thread cpp/src/parquet/encryption/crypto_factory.h Outdated
Comment thread python/pyarrow/tests/parquet/test_external_encryption.py Outdated
Comment thread python/pyarrow/tests/parquet/test_external_encryption.py Outdated
Comment thread python/pyarrow/tests/parquet/test_external_encryption.py Outdated
factory = pe.CryptoFactory(kms_client_factory)
result = factory.external_file_encryption_properties(kms_config, external_encryption_config)

# Instead of isinstance, check class name and module dynamically because ExternalEncryptionConfiguration is not visbile
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean ExternalFileEncryptionProperties?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, is nothing accessible? If we have access to the result what does it contain?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly — the ExternalFileEncryptionProperties class is not directly accessible in Python, only through the external_file_encryption_properties method in CryptoFactory. If you see that we’re going to need instances of ExternalFileEncryptionProperties in Python, we would need to add the class in python/pyarrow/parquet/encryption.py, but I don’t think so, since even FileEncryptionProperties doesn’t work that way.

"""Ensure new encryption_algorithm is accepted."""

pe.ExternalEncryptionConfiguration(
footer_key="key",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no real test here, you're not asserting anything, just checking no exceptions are thrown I guess. I would remove this test and instead up in your external encryption config change one of the column's algorithms to EXTERNAL_DBPA_V1 and do an assert that it is correctly built

@cristekdatum cristekdatum merged commit 7695544 into dev_phase2 Aug 11, 2025
20 of 48 checks passed
@cristekdatum cristekdatum deleted the 26 branch August 12, 2025 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants