IQSS/6993-use-single-AWS-client-per-store #6994
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
also refactor S3StorageIO to re-use single client per store, use more static methods
Nominally useful if code ever changes the storageIdentifier of the dvObject after the S3AccessIO instance is created, but real code shouldn't do that :-)
and the dataset pid being set multiple times.
for directUpload case
used in DatasetPage and editFilesFragment.xhtml
Conflicts: src/main/webapp/resources/js/fileupload.js
Conflicts: src/main/java/edu/harvard/iq/dataverse/DatasetPage.java src/main/webapp/resources/js/fileupload.js
keeping the cancel delay at 1 second - any queued uploads need to finish.
Conflicts: src/main/java/edu/harvard/iq/dataverse/api/Datasets.java
djbrooke
added this to Code Review 🦁
in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34)
via automation
landreev
approved these changes
Aug 20, 2020
There was a problem hiding this comment.
Looks good.
@qqmyers could you please sync up the branch with develop? - There are are no conflicts, but updating the version in pom.xml would help with QA deployments.
Everything appears to work. I was also curious to measure how much exactly we are saving per access call... Got strange results, then realized my experiment was faulty... Then decided not to hold the PR on account of it. So maybe I can instead help with that during QA (since it really is a QA task).
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34)
automation
moved this from Code Review 🦁
to QA 🔎✅
Aug 20, 2020
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34)
automation
moved this from QA 🔎✅
to Done 🚀
Aug 24, 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.
What this PR does / why we need it: Refactors the S3AccessIO class to avoid creating a new AWS client for each instance, and instead keeping track of one AWS client per store.
Which issue(s) this PR closes:
Closes #6993
Special notes for your reviewer:
Suggestions on how to test this: This should do nothing :-) - behavior should be the same (except lower memory use perhaps). Since it's S3-related, testing should focus on getting/putting files to S3. Nominally, since the change is related to keeping track of S3 stores correctly, testing should probably use a couple stores and verify that read/writes are still happening to the correct store/bucket.
Does this PR introduce a user interface change? If mockups are available, please link/include them here: No
Is there a release notes update needed for this change?: No
Additional documentation: