Conversation
Error was: LinkedWiki configuration error MediaWiki is unable to load the extension LinkedWiki. Please check that the extension's name is correct and all of its files are properly installed.
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughUpdates docker-compose to use production images (1.46), adds an Apache-based runtime split (apache + php-fpm-wikibase) with shared volumes and WIKIBASE_HOST env, and removes the LinkedWiki MediaWiki LocalSettings configuration. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docker-compose.yml`:
- Around line 86-88: The WIKIBASE_HOST env var must remain the public hostname
(used by Traefik labels and LocalSettings.d) instead of the internal FPM
container name; change the docker-compose service to stop overriding
WIKIBASE_HOST with "php-fpm-mardi-wikibase" and introduce a new variable (e.g.,
PHP_FPM_HOST) set to that internal container name; update any places that
reference the internal FPM upstream (Traefik labels, proxy upstreams, or
LocalSettings.d references to the FPM host) to use PHP_FPM_HOST while leaving
WIKIBASE_HOST as the public host so Traefik routing and canonical URLs keep
working.
| <<: *wikibase_variables | ||
| WIKIBASE_HOST: php-fpm-mardi-wikibase | ||
| labels: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's verify the file structure
git ls-files | grep -E "(docker-compose|LocalSettings)" | head -20Repository: MaRDI4NFDI/portal-compose
Length of output: 830
🏁 Script executed:
# Run the verification script provided in the review
rg -n "WIKIBASE_HOST|PHP_FPM_HOST" docker-compose.ymlRepository: MaRDI4NFDI/portal-compose
Length of output: 1479
🏁 Script executed:
# Check if LocalSettings.d directory exists and search it
if [ -d "mediawiki/LocalSettings.d" ]; then
rg -n "WIKIBASE_HOST|PHP_FPM_HOST" mediawiki/LocalSettings.d
else
find . -name "LocalSettings*" -type f 2>/dev/null | head -10
fiRepository: MaRDI4NFDI/portal-compose
Length of output: 688
🏁 Script executed:
# Let's see the full docker-compose.yml file context around lines 86-88
wc -l docker-compose.yml
head -100 docker-compose.yml | tail -30Repository: MaRDI4NFDI/portal-compose
Length of output: 1594
🏁 Script executed:
# Look at the php-fpm-wikibase service definition
sed -n '95,120p' docker-compose.ymlRepository: MaRDI4NFDI/portal-compose
Length of output: 1087
🏁 Script executed:
# Check for Apache configuration files
find . -name "*.conf" -o -name "apache*" -type f 2>/dev/null | grep -E "(apache|php)" | head -20Repository: MaRDI4NFDI/portal-compose
Length of output: 51
🏁 Script executed:
# Check if there's any mention of FPM upstream in config files
rg -l "FPM|fastcgi|upstream" --type conf --type phpRepository: MaRDI4NFDI/portal-compose
Length of output: 97
🏁 Script executed:
# Check docker-compose for wikibase service definition to confirm
sed -n '60,95p' docker-compose.ymlRepository: MaRDI4NFDI/portal-compose
Length of output: 1512
🏁 Script executed:
# Look for any Apache or PHP-FPM configuration in mediawiki directory
find mediawiki -type f \( -name "*.conf" -o -name "*.conf.d" \) 2>/dev/nullRepository: MaRDI4NFDI/portal-compose
Length of output: 51
🏁 Script executed:
# Check if there's any FastCGI or FPM configuration in LocalSettings.d files
rg -n "fastcgi|FastCGI|FPM|fpm|php-fpm" mediawiki/Repository: MaRDI4NFDI/portal-compose
Length of output: 51
Don't override WIKIBASE_HOST with the internal FPM container name—it breaks Traefik routing and canonical URLs.
The current setup at line 87 immediately breaks the Traefik routing labels in the same service (lines 89, 94): they will attempt to route traffic to Host(php-fpm-mardi-wikibase, swmath.php-fpm-mardi-wikibase) instead of the public hostname. Additionally, this breaks all LocalSettings.d configurations that depend on WIKIBASE_HOST for canonical URLs, email (IDHost), VisualEditor, Matomo Analytics, and Quickstatements.
Keep WIKIBASE_HOST as the public host and introduce a separate PHP_FPM_HOST variable for the internal FPM upstream connection.
💡 Suggested change
environment:
<<: *wikibase_variables
- WIKIBASE_HOST: php-fpm-mardi-wikibase
+ # keep public host for canonical URLs and Traefik routing
+ PHP_FPM_HOST: php-fpm-mardi-wikibase🤖 Prompt for AI Agents
In `@docker-compose.yml` around lines 86 - 88, The WIKIBASE_HOST env var must
remain the public hostname (used by Traefik labels and LocalSettings.d) instead
of the internal FPM container name; change the docker-compose service to stop
overriding WIKIBASE_HOST with "php-fpm-mardi-wikibase" and introduce a new
variable (e.g., PHP_FPM_HOST) set to that internal container name; update any
places that reference the internal FPM upstream (Traefik labels, proxy
upstreams, or LocalSettings.d references to the FPM host) to use PHP_FPM_HOST
while leaving WIKIBASE_HOST as the public host so Traefik routing and canonical
URLs keep working.
|
It is hard to tell whether this will work as expected because our docker-wikibase images have been rewritten for a Kubernetes environment. All the LocalSettings volumes, for instance, should now be deleted because they are already part of the image and don't need to be shared around. I set up a simple version of all that for development purposes where you can see that there are no volumes: https://github.com/MaRDI4NFDI/portal-compose/blob/main/docker-compose-dev.yml WIKIBASE_HOST should now refer to the container that runs the mediawiki code. In this setup you have called this container php-fpm-mardi-wikibase so it is correct to tell the apache container that WIKIBASE_HOST = php-fpm-mardi-wikibase so that it knows where to pass the requests. You are right that the assets in https://portal.mardi4nfdi.de/w/resources/assets/ should be in the apache container. In Kubernetes we do this with an initContainer that just copies them from the wikibase image https://github.com/MaRDI4NFDI/portal-k8s/blob/95b2c38e08d8f5a51e229e515b3ec0e2d18af3a9/charts/wikibase/templates/deployment-apache.yaml#L24 but in this setup with docker compose you probably have to setup a volume for the apache image. |
I bet it will and as you can see at https://mardi.schubotz.org/ it looks good so far.
Yeah for the future. However, I think we should sunset the use of docker-compose for the MaRDI project entirely. For now the goal was to update the mw version without changing a lot.
Do you think this variable name is optimal? I would rather suggest changing that to something which indicates that this should be pointed to the fpm instance
Why is that? Would it not be more natural to do this when to container is build? |
|
https://mardi.schubotz.org/ looks good so far because it is running with a very old image. What I meant is that I am not sure it will work once you deploy a kubernetes image. I guess WIKIBASE_HOST can be changed, but it is the name we had always used and it would have to be changed in lots of places. The files copied in the apache image are different for the staging and the production environment. For instance |
No if you go to mardi.schubotz.org/wiki/Special:Version you see that it has the current image:-)
I would disagree. WIKIBASE_HOST did always point to the apache host. Only the php process that did run within that container now runs on a different container. Therefore we should use a new variable. @eloiferrer I think it's important for me to understand which one is the new WIKIBASE_HOST. From how I understand the topology the php-fpm image is just a helper. |
|
Now I see the logo and other assets. |
2 participants
Open problems
This pull request updates the
docker-compose.ymlto refactor the Wikibase service setup and update several container images. The main change is splitting the Wikibase service into separate Apache and PHP-FPM containers, improving maintainability and flexibility. Additionally, the pull request updates image references for better consistency and removes theLinkedWikiextension configuration.Service architecture refactor:
php-fpm-wikibaseservice using the updatedwikibase-productionimage, sharing configuration and data volumes with other services.wikibaseservice to use a dedicated Apache image, separating web server and PHP logic. [1] [2]Image updates:
redis-jobrunnerandwikibaseto use their respectiveproduction:1.46tags for improved stability and consistency.Configuration and volume changes:
images_repodirectory as read-only in the Apache container and added theWIKIBASE_HOSTenvironment variable for better routing and environment management.Extension removal:
LinkedWikiextension and its configuration frommediawiki/LocalSettings.d/LinkedWiki.php, simplifying the MediaWiki setup.This is deployed for testing at https://mardi.schubotz.org
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.