-
Notifications
You must be signed in to change notification settings - Fork 9k
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
[HADOOP-13075] Adding support for SSE-KMS and SSE-C #113
Conversation
* The standard encryption algorithm AWS supports. | ||
* Different implementations may support others (or none). | ||
*/ | ||
public static final String SERVER_SIDE_ENCRYPTION_AES256 = |
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.
why was this cut? This is directly referred to in {{TestS3AEncryption}}, a file which this patch doesn't touch. I don't think a clean build of this patch is going to work. Have you tried it?
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.
As you said, that constant is only used in that test which I did change. I changed it to abstract and created 3 different implementations: one for SSE-S3, SSE-KMS and SSE-C. Basically I'm running all the tests in TestS3AEncryption, but with different encryption algorithms depending on the concrete class.
Yes, it builds and all test pass.
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.
we can't pull values out of public interfaces like this, even if its not supported. They'd need to be tagged as deprecated
Hello, what's the status of this patch? Would be really great if we could use S3 SSE-C with Hadoop on EMR. :) |
@elad best to discuss that on the JIRA itself; FWIW, EMR's S3 connector is Amazon's codebase, so what we do in the ASF doesn't have any impact there. We are striving to be better :-) Status: we've gone and broken it in Hadoop branch-2 by replacing @fedecz —could you bring this up to sync with branch-2 and I'll add reviewing it to my todo list. thanks |
Can one of the admins verify this patch? |
HADOOP-13075 has been fixed. Closing. |
…InterruptInRetryLoop It's possible that the interruptee thread (see `#interruptedThread`) gets pre-empted before it has a chance to run the operation (increment `iterations`) and then gets interrupted, causing these assertions to fail. I think these assertions also aren't critical for the tests which I presume want to test interrupt propagation behavior, so removing them in this change. vjagadish1989 & xinyuiscool, please take a look. Author: Prateek Maheshwari <pmaheshw@linkedin.com> Reviewers: Jagadish <jagadish@apache.org> Closes apache#113 from prateekm/flaky-ess-test-fix
No description provided.