Resources S3 storage optimization #902
Merged
Conversation
This file contains 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
adamkingit
reviewed
May 27, 2021
| return new S3LegacyResourceHandler(decodeURIComponent(path), bucketName, urlString); // endpoint from urlString is not used | ||
| }; | ||
|
|
||
| module.exports = { constructor, deserializer }; |
There was a problem hiding this comment.
Suggested change
| module.exports = { constructor, deserializer }; | |
| module.exports = { constructor, deserializer }; | |
adamkingit
reviewed
May 27, 2021
| const data = this.targetHandler.serialize(); | ||
| return { metadata: this.metadata, data }; | ||
| } | ||
| }; |
adamkingit
reviewed
May 27, 2021
| return new S3ResourceHandler(data.path, data.bucketName, data.location, data.endpoint); | ||
| }; | ||
|
|
||
| module.exports = { constructor, deserializer }; |
There was a problem hiding this comment.
Suggested change
| module.exports = { constructor, deserializer }; | |
| module.exports = { constructor, deserializer }; | |
adamkingit
reviewed
May 27, 2021
|
|
||
| }); | ||
|
|
||
| }); |
There was a problem hiding this comment.
Suggested change
| }); | |
| }); | |
adamkingit
reviewed
May 27, 2021
| console.log('All done!'); | ||
| } | ||
|
|
||
| test().catch(console.log); |
There was a problem hiding this comment.
Suggested change
| test().catch(console.log); | |
| test().catch(console.log); | |
adamkingit
reviewed
May 27, 2021
| } | ||
| } | ||
|
|
||
| module.exports = new StorageFactory(); |
There was a problem hiding this comment.
Suggested change
| module.exports = new StorageFactory(); | |
| module.exports = new StorageFactory(); | |
adamkingit
reviewed
May 27, 2021
| "s3": "app/storage/s3ResourceHandler", | ||
| "embedded": "app/storage/embeddedResourceHandler", | ||
| "s3legacy": "app/storage/legacyS3ResourceHandler" | ||
| } |
adamkingit
reviewed
May 27, 2021
|
|
||
| } | ||
|
|
||
| test().catch(console.log); |
There was a problem hiding this comment.
Suggested change
| test().catch(console.log); | |
| test().catch(console.log); | |
adamkingit
reviewed
May 27, 2021
| return new EmbeddedResourceHandler(data); | ||
| }; | ||
|
|
||
| module.exports = { constructor, deserializer }; |
There was a problem hiding this comment.
Suggested change
| module.exports = { constructor, deserializer }; | |
| module.exports = { constructor, deserializer }; | |
adamkingit
reviewed
May 27, 2021
| } | ||
| */ | ||
|
|
||
| module.exports = { encrypt, decryptBuffer }; // encryptStream, decryptStream |
There was a problem hiding this comment.
Suggested change
| module.exports = { encrypt, decryptBuffer }; // encryptStream, decryptStream | |
| module.exports = { encrypt, decryptBuffer }; // encryptStream, decryptStream | |
adamkingit
reviewed
May 27, 2021
|
|
||
| const encryptionAlgorithm = 'aes-256-cbc'; | ||
|
|
||
| const encrypt = (stringOrBuffer, encryptionKey) => { |
There was a problem hiding this comment.
Is this new encryption, or a refactoring of what was already there?
There was a problem hiding this comment.
the same encryption as we have today (has to be or decryption will not work), but it was duplicated across multiple files, now it's refactored a little and decoupled from S3client.
rmgraham
previously approved these changes
May 27, 2021
carrolp
previously approved these changes
May 27, 2021
|
Added approval: combined rmgraham's earlier approval with @michael-topchiev 's approval of my added commit that dismissed rmgraham's.... == approved again. |
dalehille
previously approved these changes
May 28, 2021
dalehille
approved these changes
May 28, 2021
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.
No description provided.