-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
EZP-30835: Added support for Solr 7.7 #136
Conversation
Will this also pass on Solr 6.6 or we drop support for that in 1.6? |
I will check 😉 IMHO we should drop Solr 6.6 support in release with Solr 7. Changelog for Solr/Lucene between 6.6 and 7.X is big (includes BC breaks) which might make hard to have the support of the same feature set for both versions. |
ok 👍 |
bin/.travis/init_solr.sh
Outdated
@@ -44,7 +44,7 @@ fi | |||
download() { | |||
case ${SOLR_VERSION} in | |||
# PS!!: Append versions and don't remove old once, kernel uses this script! | |||
6.3.0|6.4.1|6.4.2|6.5.1|6.6.0|6.6.5 ) | |||
6.3.0|6.4.1|6.4.2|6.5.1|6.6.0|6.6.5|7.7.1 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: 7.7.2 is out
Also, any idea how much they have broken in 8 in terms of also testing this branch on Solr 8.2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: 7.7.2 is out
Thanks for the information! I will update PR.
Also, any idea how much they have broken in 8 in terms of also testing this branch on Solr 8.2?
Let's add 8.2 to the test matrix and we will see 😉
@adamwojs we kind of need this in 2.5 as well, people are expecting to be able to use 7.7LTS given 6.6 is starting to show its age. Another issue is Debian 10 coming with Java 11, however don't seems like any Solr version supports it yet, so it's more likely to be added in upcoming Solr 8.x release. |
29d12fb
to
d8c7400
Compare
5097963
to
31d145e
Compare
0c43666
to
ec9adee
Compare
@andrerom I discussed with @lserwatka Solr 7+ support in eZ Platform 2.5 and we decided to go as described in "Release" section in the PR description. |
Updated branch-alias for dev-master in composer.json according to #136 > Due to the major changes in Solr, support for 7.X will be part of the ezplatform-solr-search-engine 2.0 release which will be compatible with eZ Platform 2.5 release. Solr 6.X will be dropped in this release. > Solr 7.X and Solr 8.X will be officialy supported in ezplatform-solr-search-engine 3.0 release which will be compatible with eZ Platform 3.X.
Updated branch-alias for dev-master in composer.json according to #136 > Due to the major changes in Solr, support for 7.X will be part of the ezplatform-solr-search-engine 2.0 release which will be compatible with eZ Platform 2.5 release. Solr 6.X will be dropped in this release. > Solr 7.X and Solr 8.X will be officialy supported in ezplatform-solr-search-engine 3.0 release which will be compatible with eZ Platform 3.X.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general I'd see README.md updated as well, to remember which version of Solr Bundle supports which Solr engine version.
bin/.travis/init_solr.sh
Outdated
@@ -44,7 +44,7 @@ fi | |||
download() { | |||
case ${SOLR_VERSION} in | |||
# PS!!: Append versions and don't remove old once, kernel uses this script! | |||
6.3.0|6.4.1|6.4.2|6.5.1|6.6.0|6.6.5 ) | |||
6.3.0|6.4.1|6.4.2|6.5.1|6.6.0|6.6.5|7.7.2 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we decided to drop support for Solr 6.x
in 2.0
of this bundle, we probably should remove them here as well, to avoid suggestion we still support it.
bin/.travis/init_solr.sh
Outdated
|
||
files+=("${config_dir}/currency.xml") | ||
if [[ $SOLR_VERSION =~ ^(7) ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this check if we support 7.x only now?
bin/.travis/init_solr.sh
Outdated
files+=("${config_dir}/elevate.xml") | ||
files+=("${config_dir}/solrconfig.xml") | ||
|
||
if [[ ! $SOLR_VERSION =~ ^(7) ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same question here
bin/generate-solr-config.sh
Outdated
cp ${SOLR_INSTALL_DIR}/server/solr/configsets/basic_configs/conf/{currency.xml,solrconfig.xml,stopwords.txt,synonyms.txt,elevate.xml} $DESTINATION_DIR | ||
|
||
if [[ $SOLR_VERSION =~ ^(7|8) ]]; then | ||
cp ${SOLR_INSTALL_DIR}/server/solr/configsets/_default/conf/{solrconfig.xml,stopwords.txt,synonyms.txt} $DESTINATION_DIR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides previous comments it looks good to me, great job @adamwojs!
PR updated according to @alongosz code review suggestions. |
composer.json
Outdated
@@ -12,7 +12,7 @@ | |||
], | |||
"require": { | |||
"php": "^7.1", | |||
"ezsystems/ezpublish-kernel": "^7.5@dev", | |||
"ezsystems/ezpublish-kernel": "dev-solr_77 as 7.5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Friendly reminder ;)
Everything's fine for me, QA approve. |
PR has been rebased and squashed. |
Requires ezsystems/ezpublish-kernel#2775 for tests to pass, so merging now. |
@adamwojs could you merge up? |
Done. |
Related SOLR issues
https://issues.apache.org/jira/browse/SOLR-11501:
Ref. https://github.com/ezsystems/ezplatform-solr-search-engine/pull/136/files#diff-d75c9ba9fe393b04edf2db8762aa77b7L65
https://issues.apache.org/jira/browse/SOLR-10503:
Dropped as is not used by eZ Platform
https://issues.apache.org/jira/browse/SOLR-10729:
Will be processed as a separate task: https://jira.ez.no/browse/EZP-30977
https://issues.apache.org/jira/browse/SOLR-?????:
Replaced according to suggestion
https://issues.apache.org/jira/browse/SOLR-12770:
This is CI only related issue
https://issues.apache.org/jira/browse/SOLR-?????:
Ref. https://lucene.apache.org/solr/guide/7_2/major-changes-in-solr-7.html
Release
Due to the major changes in Solr, support for 7.X will be part of the
ezplatform-solr-search-engine
2.0 release which will be compatible with eZ Platform 2.5 release. Solr 6.X will be dropped in this release.Solr 7.X and Solr 8.X will be officialy supported in
ezplatform-solr-search-engine
3.0 release which will be compatible with eZ Platform 3.X.TODO
bin/generate-solr-config.sh
schema.xml