-
Notifications
You must be signed in to change notification settings - Fork 8.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-18965. ITestS3AHugeFilesEncryption failure #6261
HADOOP-18965. ITestS3AHugeFilesEncryption failure #6261
Conversation
Change-Id: I0e826af64f09992f385ed8f5f8064f016a9084c6
🎊 +1 overall
This message was automatically generated. |
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.
+1 (non-binding) once checkstyle is addressed.
Bucket level encryption check makes sense, missed it.
could I get a review by another, e.g @ahmarsuhail ? |
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.
+1, LGTM. just a small nit for changing variable name
if (SSE_KMS.getMethod().equals(c.get(S3_ENCRYPTION_ALGORITHM))) { | ||
|
||
final String bucket = getTestBucketName(c); | ||
String kmsKey = getS3EncryptionKey(bucket, c); |
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.
nit: consider changing variable name from c to conf
updated and reran without and with encryption (SSE-KMS) s3 london
|
variable name Change-Id: I5fbd6f7c24cd4a057d9823118b15653fed898999
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.
+1, LGTM.
🎊 +1 overall
This message was automatically generated. |
Followup to: HADOOP-18850 Enable dual-layer server-side encryption with AWS KMS keys (DSSE-KMS) Contributed by Steve Loughran
Description of PR
How was this patch tested?
reran failing test suite
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?