Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Solarium to 5.1.6 #550

Merged
merged 8 commits into from
Dec 9, 2020

Conversation

beatrycze-volk
Copy link
Collaborator

The highest possible now update of Solarium. Necessary change (https://solarium.readthedocs.io/en/stable/getting-started/):

In the past, the V1 API endpoint solr was not added automatically, so most users set it as path on the endpoint. This bug was discovered with the addition of V2 API support. In almost every setup, the path has to be set to / instead of /solr with this release!

For the same reason it is a must to explicit configure the core or collection.

Since version 5.2 needed is EventDispatcher which in TYPO3 is first available in ver. 10 (https://docs.typo3.org/m/typo3/reference-coreapi/master/en-us/ApiOverview/Hooks/EventDispatcher/Index.html).

@sebastian-meyer sebastian-meyer added the 🛠 maintenance A task to keep the code up-to-date and manageable. label Dec 8, 2020
Copy link
Member

@sebastian-meyer sebastian-meyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see my comments below.

Classes/Common/Solr.php Outdated Show resolved Hide resolved
Classes/Common/Solr.php Show resolved Hide resolved
Classes/Common/Solr.php Outdated Show resolved Hide resolved
Resources/Private/Language/Labels.xml Outdated Show resolved Hide resolved
Resources/Private/Language/Labels.xml Outdated Show resolved Hide resolved
Copy link
Member

@sebastian-meyer sebastian-meyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I confused the constructor (which actually uses Solarium to establish the Solr connection) and getSolrUrl() (which is used to create new cores without using Solarium). Some of my review comments were garbage... :o/

I've added a few changes and now it works for me. Does it for you as well?

@beatrycze-volk
Copy link
Collaborator Author

Yes, it looks ok.

@sebastian-meyer sebastian-meyer merged commit e01990c into kitodo:master Dec 9, 2020
@beatrycze-volk beatrycze-volk deleted the update-solarium branch December 9, 2020 15:58
@sebastian-meyer sebastian-meyer mentioned this pull request Dec 20, 2020
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠 maintenance A task to keep the code up-to-date and manageable.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants