V2 #6
Merged
V2 #6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Known limitations - The customer cannot currently specify that they would like to encrypt with KMS and use their default key. SSEAwsKeyManagementParams does not exist in the v2 SDK and I lost the functionality in the refactor.
…se of the customer wanting to use the AWS managed CMK.
aws-rizi
suggested changes
Jul 31, 2020
aws-rizi
previously requested changes
Jul 31, 2020
adam-aws
reviewed
Aug 4, 2020
adam-aws
reviewed
Aug 4, 2020
| @@ -212,4 +182,18 @@ public boolean isAlwaysThroughS3() { | |||
| public void setAlwaysThroughS3(boolean alwaysThroughS3) { | |||
| this.alwaysThroughS3 = alwaysThroughS3; | |||
| } | |||
|
|
|||
| public PayloadStorageConfiguration withServiceSideEncryption(ServerSideEncryptionStrategy serverSideEncryptionStrategy) { | |||
There was a problem hiding this comment.
Can we get some comments on these new methods
There was a problem hiding this comment.
Done, I've also added a description of the usage in the class docs.
Adding Javadocs to the class and methods.
eddy-aws
reviewed
Aug 14, 2020
| * This is optional, it is set only when you want to configure S3 server side encryption with KMS. | ||
| * | ||
| * @param serverSideEncryptionStrategy The method of encryption required for S3 server side encryption with KMS. | ||
| * @return |
eddy-aws
reviewed
Aug 14, 2020
| @@ -0,0 +1,11 @@ | |||
| package software.amazon.payloadoffloading; | |||
|
|
|||
| public class ServerSideEncryptionFactory { | |||
There was a problem hiding this comment.
Can we add unit tests for the newly added SseStrategy implementations?
Adding Javadocs.
eddy-aws
approved these changes
Aug 17, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
You didn't ask for it, but it enables future work.
Description of changes:
I've refactored the project to use the AWS SDK for Java v2.
I doing this I have changed how the configuration of server side encryption is managed. I've implemented it in one way, but I'm sure there could be better ways.
This isn't a final cut, I'm missing documentation and unit test coverage.
I thought it best to get the ball rolling on what is quite a big PR.
Thanks,
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.