-
Notifications
You must be signed in to change notification settings - Fork 0
External decryption config bindings #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
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? or See also: |
9973358 to
53323b9
Compare
|
It's very confusing to try to do this review with all the pending changes for the other 2 PRs. Please rebase properly and send out for review again showing only the diffs relevant to this PR. Thanks |
… external_decryption_config_bindings
518a9bb to
7e44f69
Compare
7e44f69 to
24f7f2a
Compare
… external_decryption_config_bindings
f8e7554 to
6674986
Compare
6674986 to
457af33
Compare
sofia-tekdatum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
| for cipher_name, inner_dict in value.items(): | ||
| cipher_enum = cipher_from_name(cipher_name) | ||
| if not isinstance(inner_dict, dict): | ||
| raise TypeError(f"Inner value for cipher {cipher_name!r} must be a dict") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the "!r"
Based on the changes for #31 and the task #33 there are the new ExternalDecryptionConfiguration bindings