-
Notifications
You must be signed in to change notification settings - Fork 86
Examples refresh, take 1 #219
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
Examples refresh, take 1 #219
Conversation
* Added a check for max_age being greater than 0 * Fixed flake8 by adding missing pydocstyle dependency * Added the dependency to decrypt_oracle as well * Added test for max_age<=0 ValueError * Updated test for max_age<=0.0 ValueError * Added negative test case
Update PR template
…yption-sdk-python into a1b1c1-example
… still working on populating the tests correctly, so the CI will fail, but I tested the code with my own KMS CMK ARNs, so I know it will work once the tests are populated (working with Tejeswini on this)
…d test keys in different regions for this example
…ured (#179) * Fixed KMS master key provider tests for users who have their default AWS region configured * created fixture for botocore session with no region set * add auto-used fixture in KMS master key provider unit tests to test against both with and without default region
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.
Got through the intro + streaming example. If you accept a change, please propagate that change throughout the file. I'll review again when you've rev'd the PR.
for chunk in encryptor: | ||
ciphertext.write(chunk) | ||
|
||
# Verify that the ciphertext and plaintext are different. |
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.
Kind of a low bar ...
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.
It is, but I'm not sure what else we can really make a concrete statement about to demonstrate that "yes, this is encrypted" without getting really complicated.
Co-Authored-By: June Blender <[email protected]>
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.
Reviewed until MK/MKP legacy examples. If you need more, let me know.
Co-Authored-By: June Blender <[email protected]>
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 good!
* Adding new examples and example test runner to follow the format set in aws/aws-encryption-sdk-python#219 * Updated wording and copyright notice * Adding periods * Adding NIST recommendation for RSA * Adding example for DER formatted RSA keys * Wording changes based on feedback
Issue #, if available:
#156
#213
resolves: #177
Description of changes:
There are three main things going on here:
legacy
module. I'll be pulling some of them out into their appropriate place later, pending the next item.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Check any applicable: