Configurable docroot via MicroProfile Config #9819
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
The default from microprofile-config.properties does NOT work, as the location must already be resolvable while the servlet is being initialized - the app shipped defaults file is not yet read at this point. This is similar to the database options, which must be set using one of the other Payara included config sources. (Non-easily resolvable timing issue). The solution for containers is to add an env var to the docker file, which can be overriden by any env var from compose or K8s etc. (Problem is the high ordinal of the env source though)
Instead of trying to provide a default using STORAGE_DIR env var from microprofile-config.properties as before, using this env var reference in glassfish-web.xml directly now. By defaulting to "." if not present (as in classic installations), it is fully equivalent to the former hardcoded default value. Providing a synced variant of it in microprofile-config.properties and leaving a hint about the pitfalls, we can reuse the setting for other purposes within the codebase as well (and expect the same behaviour because same defaults).
…IQSS#9572 Starting with important local storage locations for the Dataverse application, this service uses EJB startup mechanisms to verify configuration bits on startup. Checks for the temp storage location and JSF upload location as crucial parts of the app, which, if not exist or write protected, while only cause errors and failures on the first data upload attempt. This is not desirable as it might cause users to be blocked.
Add JVM Setting and add to config checker on startup to ensure target location is in good shape.
…figuration checking
…ocument more of the behaviour
By introducing a new static method ThemeWidgetFragment.getLogoDir all other places (api.Access, api.Dataverse, UpdateDataverseThemeCommand, DataverseServiceBean) can use a lookup function from one central place instead of building the path on their own. Reducing code duplication also means we can more easily get the location from a setting, enabling relocation of the data. That is especially important for container usage. Also, we can now use ConfigCheckService to detect if the folders we configured are read/write accessible to us.
…IQSS#9662 Payara does not support looking up variables in default values of a lookup. As a consequence, we must return to the hardcoded "./docroot" and "./uploads" and instead supply default values using two environment variables in the applications' Dockerfile. This way they stay configurable from cmdline or other sources of env vars.
With Files.notExist if some folder does not have the "execute" attribute, it cannot detect a folder does not exist. Inverting the Files.exists call solves the problem. Also adding tests for the business logic.
poikilotherm
added
Feature: Installation Guide
Component: Containers
poikilotherm
changed the title
This is also used in ConfigCheckServiceTest to verify the checks are working. This property is picked up when sitemap util looks up the storage location via MPCONFIG, parsing the default values during testing from src/test/resources/META-INF/microprofile-config.properties
By providing a sane default unter /tmp, we enable a few tests that do not use a custom testclass scoped directory to run
- No more profile to work around Payaras bug with overriding profiled values - Group together because every item using $STORAGE_DIR and a default to match classic installs now
bencomp
reviewed
Sep 14, 2023
src/test/java/edu/harvard/iq/dataverse/sitemap/SiteMapUtilTest.java
Outdated
Show resolved
Hide resolved
poikilotherm
force-pushed
the
9662-docroot-mpc
branch
from
d3ac6ff to
efaf5d5
Compare
|
2023/09/18: Moved to Sprint Ready as per discussion in prioritization meeting. |
15 tasks
|
@landreev I just merged |
landreev
approved these changes
Oct 4, 2023
pdurbin
added a commit
that referenced
this pull request
Oct 18, 2023
Jenkins is failing. Maybe disabling this will help. It was added in this PR: #9819
pdurbin
added a commit
that referenced
this pull request
Oct 26, 2023
Jenkins is failing. Maybe disabling this will help. It was added in this PR: #9819
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:
This pull request solves the problem that inside of containers you want to store anything underneath docroot etc on a different, usually bind-mounted filesystem. This PR makes the necessary code changes to
Which issue(s) this PR closes:
Special notes for your reviewer:
None
Suggestions on how to test this:
Watch
/dvand its subfolders when starting a container env viamvn -Pct package docker:runand storing data like fileuploads, collection themes, etc.Does this PR introduce a user interface change? If mockups are available, please link/include them here:
None.
Is there a release notes update needed for this change?:
Maybe? Please leave a comment.
Additional documentation:
None.