7000 mpconfig files basic #8827
Conversation
poikilotherm
added
Feature: File Upload & Handling
Component: Code Infrastructure
poikilotherm
force-pushed
the
7000-mpconfig-files-basic
branch
from
066d059 to
b0b96b3
Compare
poikilotherm
force-pushed
the
7000-mpconfig-files-basic
branch
from
b0b96b3 to
8ac60bb
Compare
… MPCONFIG IQSS#7000 - Adding dataverse.files.directory equivalent to JvmSettings - Remove all System.getPropert("dataverse.files.directory") or similar - Add default with /tmp/dataverse via microprofile-config.properties as formerly seen at FileUtil and Dataset only - Refactor SwordConfigurationImpl to reuse the NoSuchElementException thrown by MPCONFIG - Refactor GoogleCloudSubmitToArchiveCommand to use the JvmSettings.lookup and create file stream in try-with-resources
poikilotherm
force-pushed
the
7000-mpconfig-files-basic
branch
from
8ac60bb to
d7ab9f6
Compare
...ain/java/edu/harvard/iq/dataverse/engine/command/impl/GoogleCloudSubmitToArchiveCommand.java
Show resolved
Hide resolved
There was a problem hiding this comment.
More conversions to microprofile. The documentation here raises an issue as to whether we have out-of-date info elsewhere in the guides (re: the dir to add files to when doing an import) that should be fixed somewhere (with the docs being added here corrected to match).
FWIW: @scolapasta, @kcondon, and I had a discussion about how to QA these mp conversions since they are all repeating the same pattern. I think the outcome was to just check that a JVMoption/mpconfig file, env variable works once on some PR and for the rest of the conversions just test that one mode works (i.e. the existing jvm option today) - versus checking that all options work for every setting that is updated in every PR.
|
added to sprint Dec 15, 2022 |
|
@poikilotherm heads up that this PR has merge conflicts. |
|
All set - forgot to push the merge.... |
|
@poikilotherm as expected, more merge conflicts as other PRs are merged. Can you please resolve them? |
|
@poikilotherm merge conflicts and it looks like there are changes requested by @qqmyers. Can you please take a look? Update: I checked with Oliver about this and he said we should discuss it next year. |
|
Sizing:
|
|
🥳 another one merged! Thanks for keeping up the pace @kcondon |
What this PR does / why we need it:
Replace all lookups of the
dataverse.files.directorywith MPCONFIG and provide a sane default/tmp/dataverse(as seen with some of the former usages). Also add lots of detail docs to the config guide about this setting.This PR is fully backward compatible with any existing configurations as it does no renaming or behaviour changes (except for the default, but every installation out there is very likely not relying on a default for this setting!).
Relates to #7000
Which issue(s) this PR closes:
None.
Special notes for your reviewer:
It is the next simple example to see how the pieces laid out in #8823 are coming together.
Suggestions on how to test this:
Play around with the settings described in the little dev docs section and watch it work.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Not really.
Is there a release notes update needed for this change?:
Maybe? Dunno. Please leave comment. IMHO this is very much dev-only.
Additional documentation:
Included.