feat(upload): make upload file storage path configurable #6656#8983
Merged
kcondon merged 6 commits intoIQSS:developfrom Dec 5, 2022
Merged
feat(upload): make upload file storage path configurable #6656#8983kcondon merged 6 commits intoIQSS:developfrom
kcondon merged 6 commits intoIQSS:developfrom
Conversation
As outlined in IQSS#6656, files will be stored in `domaindir/generated/jsp/dataverse` during upload before being moved to our temporary ingest file space at `$dataverse.files.directory/temp`. With this commit, we enable to configure a different place for these kind of generated temporary files by using MPCONFIG variable substitution inside of glassfish-web.xml. Also sorts the content of glassfish-web.xml into order as specified by the XSD. Documentation of the setting is provided.
This was referenced Sep 30, 2022
poikilotherm
added a commit
to poikilotherm/dataverse
that referenced
this pull request
Nov 4, 2022
- Instead of a /docroot, add a more generic /dv which is owned by payara:payara and can be used to either store data in a single volume using subfolders or use subfolders with different backing volumes. Anyway, data is not written to overlay FS this way. (As long as an app image points to this location) - Also define /secrets and /dumps as volumes, so data flowing into these locations is again not added to the overlay FS (which might cause severe damage in case of heap dumps!) - Document the different locations in the base image guide. - Remove the /docroot workaround for uploaded files. This will be solved at application level (either by moving the workaround there) or IQSS#8983
Contributor
Author
Contributor
|
This looks great, we should be able to move it along and merge it in the next sprint. |
Member
|
Small code change. Good docs. Gave it a size of 33. |
pdurbin
approved these changes
Dec 2, 2022
Member
There was a problem hiding this comment.
Looks great! Thanks, @poikilotherm!! Approved (I made small doc changes). @landreev also made a positive comment: #8983 (comment)
The only testing I did was to confirm that the new unconfigured JSF upload default directory is /usr/local/payara5/glassfish/domains/domain1/uploads. I assuming configuration works fine.
Moving to QA.
Merged
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
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:
As outlined in #6656, files will be stored in
domaindir/generated/jsp/dataverseduring upload before being moved to our temporary ingest file space at$dataverse.files.directory/temp.With this commit, we enable to configure a different place for these kind of generated temporary files by using MPCONFIG variable substitution inside of glassfish-web.xml. This is kinda hacky, but at least it allows to store the data in distinguished places.
Also sorts the content of glassfish-web.xml into order as specified by the XSD. Documentation of the setting is provided.
Which issue(s) this PR closes:
Closes #6656
Special notes for your reviewer:
Please see screenshots below. You can also try with
docker-aio.I added a few notes about temporary upload storage to the configuration guide with the gist of knowledge from #6656 so admins can learn about potential problems with large file uploads.
The container base image in #8933 certainly will include something using this if this PR would be merged before. Otherwise I'd extend this PR to include something for the container scenario.
Suggestions on how to test this:
Configure your deployment with a system property pointing
dataverse.files.uploadsto a different place and watch data being stored there.Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Nope.
Is there a release notes update needed for this change?:
Do we need one? This might be a scenario for admins as outlined by a bunch of folks. Please leave a comment.
Additional documentation:
🔋 included.