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
Make plaintext
and ciphertext
optional for batch operations
#1049
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1049 +/- ##
==========================================
+ Coverage 85.00% 85.02% +0.01%
==========================================
Files 65 65
Lines 3135 3139 +4
==========================================
+ Hits 2665 2669 +4
Misses 470 470
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
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 again for putting up this PR and especially for adding tests. Could you rebase this to include the latest changes in main
?
The Vault API requires setting `plaintext` and `ciphertext` for the encrypt and decrypt API, even though they are ignored when `batch_input` is set. This means that end users have to include an empty argument when doing batch operations, which is a bit annoying.
5adc496
to
71b49d2
Compare
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.
Looks great, thanks again @dosisod ! We're actively looking for contributors so if you're interested please don't be a stranger :)
The Vault API requires setting
plaintext
andciphertext
for the encrypt/decrypt data API, even though the fields are ignored whenbatch_input
is set. This means that end users have to include an empty argument when doing batch operations, which is a bit annoying.While this works, it might be more ergonomic to have an
encrypt_data
and abatch_encrypt_data
function as most of theencrypt_data
args are ignored in batch mode anyways, which could potentially lead to bugs. This is a bit out of scope for this PR, just thought I would mention it here as an alternative.Also, I could not test locally due to self-signed certificate errors. Is there a certificate I need to install for testing?