7000 mpconfig solr #8825
Merged
7000 mpconfig solr #8825
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
poikilotherm
added
Component: Code Infrastructure
This was referenced Jun 30, 2022
poikilotherm
commented
Jul 6, 2022
src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java
Outdated
Show resolved
Hide resolved
poikilotherm
force-pushed
the
7000-mpconfig-solr
branch
from
d495b53 to
5d03cbc
Compare
…#7000 By refactoring SystemConfig.getSolrHostColonPort, the Solr endpoint is not just configurable via a database setting, but also by all mechanisms of MicroProfile Config. - The database setting still has priority over the other mechanisms. - It's completely backward compatible, no config change necessary. - Tests have been added to ensure the behaviour - Default ("localhost:8983") for no setting given is now also done via MPCONFIG - Default for container usage ("solr:8983") possible via MPCONFIG profile "ct"
When using Dataverse with a non-default Solr, HTTPS, custom core name or similar, it's necessary to have a configurable URL for the Solr endpoint. This becomes now possible via MicroProfile Config, defaulting to the old variant.
Describe the new options to set the Solr endpoint, crosslinking the old way and adding hints about MPCONFIG profiles.
poikilotherm
force-pushed
the
7000-mpconfig-solr
branch
from
5d03cbc to
6965beb
Compare
|
added to sprint Dec 15, 2022 |
pdurbin
reviewed
Dec 19, 2022
src/main/java/edu/harvard/iq/dataverse/search/SolrClientService.java
Outdated
Show resolved
Hide resolved
As we are now using JvmSettings everywhere as an abstraction above MPCONFIG, no need to import on our own.
pdurbin
approved these changes
Dec 20, 2022
|
@poikilotherm more merge conflicts, as expected since other MPCONFIG PRs have been merged. Can you please resolved them? |
|
Ready to merge except for the merge conflicts. |
|
All set! 😄 |
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:
For unknown reasons, the Solr endpoint has been made configurable via a Database Setting only. This is very cumbersome for testing and container usage. Providing simple defaults via MPCONFIG takes lots of this load away. It also properly decouples creating the connection to Solr from the Database connection.
This PR is fully backward compatible with the old behaviour - anyone already having the database setting will let that setting take precedence!
This PR also very much enhances the configuration of the Solr connection, as it allows for the first time to choose transport protocol, core name and/or path, which makes it much more compatible with Solr installations that differ from our classic installations. (It does, however, not yet add support for SolrCloud).
Relates to #7000
Closes #3212
Which issue(s) this PR closes:
None.
Special notes for your reviewer:
Suggestions on how to test this:
Play around with the settings described in the added docs and watch it work. Or take a look at the nice test examples, also using the new shiny
@JvmSettingtest extension!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.
Additional documentation:
Included.