Page MenuHomePhabricator

Local (non-Commons) uploads don't work properly on PatchDemo
Closed, ResolvedPublicBUG REPORT

Description

Local wiki uploads (i.e. not Commons) currently fail to display on PatchDemo instances, where the Special:Redirect url used to link to them fails.

Acceptance criteria

Details

Other Assignee
bvibber
Related Changes in GitLab:
TitleReferenceAuthorSource BranchDest Branch
Set up wikipedia foreign file reporepos/test-platform/catalyst/ci-charts!97mlitnmainmain
Set up wikipedia foreign file reporepos/test-platform/catalyst/patchdemo!202mlitnmainmain
Customize query in GitLab

Event Timeline

SDunlap subscribed.

Two thoughts:

First, we are not super familiar with the feature you are testing nor the $wgForeignFileRepos config. Could you give us the reproduction steps to see the file error you're encountering and how setting $wgForeignFileRepos fixes the problem? Is this specifically for the "Proxy articles from wikipedia.org" setting? In that case these not "local" files, but rather proxied files from a language wikipedia?

Second, I think this will also need to be merged into ci-charts repo (for kubernetes/catalyst deployments)

First, we are not super familiar with the feature you are testing nor the $wgForeignFileRepos config. Could you give us the reproduction steps to see the file error you're encountering and how setting $wgForeignFileRepos fixes the problem? Is this specifically for the "Proxy articles from wikipedia.org" setting? In that case these not "local" files, but rather proxied files from a language wikipedia?

$wgForeignFileRepos is a setup for additional places to load image and media files from, and under the hood is how Commons is set up on production (via ForeignDBRepo and friends) or demo & third-party sites (via ForeignAPIRepo via HTTP). The default config on patchdemo I believe is with $wgUseInstantCommons on, which inserts a ForeignAPIRepo loader setup for Commons behind the scenes:
https://www.mediawiki.org/wiki/Manual:$wgForeignFileRepos
https://www.mediawiki.org/wiki/Manual:$wgUseInstantCommons

For ReaderExperiments' image browsing experiment we're using the proxy-to-wikipedia setup on patchdemo for MobileFrontContentProvider to fetch pages from English Wikipedia to test our front-end code on, and we have to sometimes manipulate URLs and data about the images.

We want to make sure the MediaWiki side sees the same image/file sources that the remote wiki that MobileFrontendContentProvider is fetching from contains, so we can manipulate them without any surprises: that means putting English Wikipedia explicitly in the lookup chain.

Without this addition, everything worked with images from Wikimedia Commons because Commons is implicitly added as a foreign API repo through the $wgUseInstantCommons wrapper config option, but files that came from English Wikipedia directly would sometimes break. We showed a breakage for this bug at for instance:

with the new config, that image should appear correctly in carousel, detail view, and visual toc

Second, I think this will also need to be merged into ci-charts repo (for kubernetes/catalyst deployments)

*nod* I'm not sure I understand the relationship between the files in test-platform and the files in patchdemo. Are they just manually synced with another patch or is there a process I should follow?

Also noting I found a PHP warning, probably harmless, with this config in local testing when I run the updater at least: T404901. Should be trivially work-aroundable by setting a dummy value for 'directory', I'll check on this.

@SDunlap Do we need anything else to move this along?

Specifically around this part:

Second, I think this will also need to be merged into ci-charts repo (for kubernetes/catalyst deployments)

*nod* I'm not sure I understand the relationship between the files in test-platform and the files in patchdemo. Are they just manually synced with another patch or is there a process I should follow?

Should we also be preparing a separate patch to ci-charts?

@SDunlap Do we need anything else to move this along?

Apologies for delay, I was supposed to followup on Thursday to gather details/add notes.

Specifically around this part:

Second, I think this will also need to be merged into ci-charts repo (for kubernetes/catalyst deployments)

*nod* I'm not sure I understand the relationship between the files in test-platform and the files in patchdemo. Are they just manually synced with another patch or is there a process I should follow?

Should we also be preparing a separate patch to ci-charts?

Yes please, should be roughly the same patch made to ci-charts:feature-proxy.php. This will ensure it works for the default backend of Patch Demo. If we merge the patch you've made already, it will work only if you uncheck the "Use (Catalyst) Kubernetes backend" checkbox. We're in the midst of a transition.

In the interim, @SDunlap will take a look at the one you made for patchdemo.

Both MRs have been merged and tested. Thank you a lot for walking us through that and making patches!