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

.platform/services.yaml: Update generate-solr-config.sh path and Solr version #61

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

adriendupuis
Copy link

@adriendupuis adriendupuis commented Feb 28, 2023

Question Answer
JIRA issue N/A
Type improvement
Target Ibexa version v4.4, v4.5, probably all v4.x
BC breaks no

Checklist:

  • Provided PR description.
  • Tested the solution manually. (Tested on Ibexa Cloud running Ibexa DXP v4.4.0 Experience and v4.4.2 Commerce)
  • Provided automated test coverage.
  • Checked that target branch is set correctly (main for features, the oldest supported for bugs).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review (ping @ibexa/engineering).

E: Error parsing configuration files:
    - services: Error loading file: while loading repository data
        'configsets/solr6' doesn't exist in the repository
          in ".platform/services.yaml", line 64, column 25

generate-solr-config.sh is actually generating config for 8.11.1 by default
https://github.com/ibexa/solr/blob/main/bin/generate-solr-config.sh#L7
@adriendupuis
Copy link
Author

The problem exist on previous v4.

https://github.com/ibexa/post-install/blob/4.4/resources/platformsh/common/4.4.x-dev/.platform/services.yaml#L56
The path is the old one to a now non-exitent file. The mainconfig is still configsets/solr6 which is inconsistent with the use of solr:7.7 image or generate-solr-config.sh generating for Solr 8.11.1 by default.

@adriendupuis adriendupuis changed the title Update generate-solr-config.sh path .platform/services.yaml: Update generate-solr-config.sh path and Solr version Feb 28, 2023
@adriendupuis adriendupuis marked this pull request as ready for review February 28, 2023 15:55
@mnocon
Copy link
Contributor

mnocon commented Apr 5, 2023

Q: The Commerce edition has Solr enabled by default (with the configsets commited), should it be updated as well?

See:
https://github.com/ibexa/post-install/blob/main/resources/platformsh/ibexa-commerce/4.5.x-dev/.platform/services.yaml#L54-L73
https://github.com/ibexa/post-install/tree/main/resources/platformsh/ibexa-commerce/4.5.x-dev/.platform/configsets/solr6/conf

@katarzynazawada
Copy link

I tested these changes on the experience edition on platform.sh - it works fine.
Now, I am waiting for information about commerce to continue testing.

@adriendupuis
Copy link
Author

Q: The Commerce edition has Solr enabled by default (with the configsets commited), should it be updated as well?

See: https://github.com/ibexa/post-install/blob/main/resources/platformsh/ibexa-commerce/4.5.x-dev/.platform/services.yaml#L54-L73 https://github.com/ibexa/post-install/tree/main/resources/platformsh/ibexa-commerce/4.5.x-dev/.platform/configsets/solr6/conf

@mnocon @katarzynazawada I would say "yes" but didn't find the time to properly test it yet.

@adriendupuis
Copy link
Author

@mnocon @katarzynazawada
I found the time to test on P.sh with a 4.4.2 Commerce and it works for me.

@adamwojs
Copy link
Member

@adriendupuis Rebase is needed here.

@katarzynazawada
Copy link

@adriendupuis I tested a case for commerce edition but without generating the config, as on commerce solr is enabled by default. It failed with error 



- services: Error loading file: while loading repository data
        'configsets/solr8' doesn't exist in the repository
          in ".platform/services.yaml", line 65, column 25



The paths have been changed from solr6 to solr8 but the config files stayed the same.

If you generate vendor/ibexa/solr/bin/generate-solr-config.sh before deploying commerce, a good config will be generated (to the solr8 directory) and it should work.

@adamwojs
Copy link
Member

adamwojs commented Jun 9, 2023

@adriendupuis Could you please address @katarzynazawada comments (and resolve conflicts) ?

@adriendupuis
Copy link
Author

This PR kind of duplicate #28 but with more progress.

Copy link
Contributor

@webhdx webhdx left a comment

Choose a reason for hiding this comment

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

All changes should be contained in 4.6.x-dev directories. We do not modify 4.6 dirs manually, I synchronize them before releases.

@adamwojs adamwojs mentioned this pull request Jun 22, 2023
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants