Page MenuHomePhabricator

ProofreadPage and PageImages share an API prefix (pi)
Closed, ResolvedPublic

Description

Both ProofreadPage and PageImages extensions use the prefix pi.

This is used in ProofreadPage for the piprop : https://en.wikisource.org/wiki/Special:ApiSandbox#action=query&format=json&meta=proofreadinfo

This is used by PyWikibot (at least) to detect the ProofreadPage namespaces and quality levels, so it can't just be changed in PRP: https://gerrit.wikimedia.org/r/plugins/gitiles/pywikibot/core/+/refs/heads/master/pywikibot/site/_extensions.py#73

This has not been a problem so far because PageImages is disabled on WS by the config:

'wmgUsePageImages' => [
	'default' => true,
	'wikibooks' => false, // T68455
	'wikisource' => false, // T68455
	'wikitech' => false,
	'lockeddown' => false,
],

Event Timeline

Inductiveload renamed this task from ProofreadPage and PageImages share an API prefix to ProofreadPage and PageImages share an API prefix (pi).Sep 8 2021, 4:13 PM

Change 719554 had a related patch set uploaded (by Inductiveload; author: Inductiveload):

[mediawiki/core@master] API prefix uniqueness: suppress for 'pi'

https://gerrit.wikimedia.org/r/719554

My proposal here is:

  • Disable the ApiPrefixUniquenessTest for the prefix pi temporarily (precedent: T196962) to unblock the CI: https://gerrit.wikimedia.org/r/719554
  • Provide a clone API endpoint with a unique prefix (prpi, say)
  • Deprecate the pi prefix and get clients to migrate to the new prefix (probably not many use this)
  • Remove the pi prefixed endpoint from ProofreadPage "at some point"
  • Finally remove the hack in https://gerrit.wikimedia.org/r/719554

Your proposal sounds good to me. Once the new prpi endpoint is in place, you can mark the pi endpoint as deprecated so we can identify clients still using it with Special:ApiFeatureUsage and nag them to update.

ProofreadPage is enabled on frrwiki (a Wikipedia) for some reason, so this essentially means it is impossible to use both modules at once as of now. I'm not sure merging the suppressing patch is a good idea for that reason (but @Legoktm already merged).

On the other hand, frrwiki is a small Wikipedia, and it doesn't look like the two endpoints are used frequently on it, so maybe not worth the work connected with changing one of the prefixes? Not sure.

spark-sql (default)> SELECT year, month, day, COUNT(*) AS api_requests FROM event.mediawiki_api_request WHERE year=2021 AND month=9 AND day=1 AND `database`='frrwiki' GROUP BY year, month, day;
year    month   day     api_requests
2021    9       1       13262
Time taken: 48.731 seconds, Fetched 1 row(s)

spark-sql (default)> SELECT year, month, day, COUNT(*) AS api_requests FROM event.mediawiki_api_request WHERE year=2021 AND month=9 AND day=1 AND `database`='frrwiki' AND params['action']='query' AND params['meta']='proofreadinfo' GROUP BY year, month, day;

year    month   day     api_requests
Time taken: 109.55 seconds

spark-sql (default)> SELECT year, month, day, COUNT(*) AS api_requests FROM event.mediawiki_api_request WHERE year=2021 AND month=9 AND `database`='frrwiki' AND params['action']='query' AND params['meta']='proofreadinfo' GROUP BY year, month, day;

year    month   day     api_requests
Time taken: 414.133 seconds

spark-sql (default)> SELECT year, month, day, COUNT(*) AS api_requests FROM event.mediawiki_api_request WHERE year=2021 AND month=9 AND day=1 AND `database`='frrwiki' AND params['action']='query' AND params['prop']='pageimages' GROUP BY year, month, day;

year    month   day     api_requests
2021    9       1       1658
Time taken: 84.804 seconds, Fetched 1 row(s)

ProofreadPage is enabled on frrwiki (a Wikipedia) for some reason, so this essentially means it is impossible to use both modules at once as of now.

Surely this has always been the case but no one has ever noticed?

Change 719554 merged by jenkins-bot:

[mediawiki/core@master] tests: suppress API prefix uniqueness check for 'pi'

https://gerrit.wikimedia.org/r/719554

ProofreadPage is enabled on frrwiki (a Wikipedia) for some reason, so this essentially means it is impossible to use both modules at once as of now.

Surely this has always been the case but no one has ever noticed?

I wouldn't be surprised by that, given the wiki is a small one and a Wikipedia having ProofreadPage is an edge case by itself.

Change 719497 had a related patch set uploaded (by Legoktm; author: Inductiveload):

[mediawiki/core@REL1_36] tests: suppress API prefix uniqueness check for 'pi'

https://gerrit.wikimedia.org/r/719497

Change 719498 had a related patch set uploaded (by Legoktm; author: Inductiveload):

[mediawiki/core@REL1_35] tests: suppress API prefix uniqueness check for 'pi'

https://gerrit.wikimedia.org/r/719498

Change 719498 merged by jenkins-bot:

[mediawiki/core@REL1_35] tests: suppress API prefix uniqueness check for 'pi'

https://gerrit.wikimedia.org/r/719498

Change 719497 merged by jenkins-bot:

[mediawiki/core@REL1_36] tests: suppress API prefix uniqueness check for 'pi'

https://gerrit.wikimedia.org/r/719497

Change 719582 had a related patch set uploaded (by Inductiveload; author: Inductiveload):

[mediawiki/extensions/ProofreadPage@master] Rename proofreadinfo API's piprop to prpiprop

https://gerrit.wikimedia.org/r/719582

ProofreadPage is enabled on frrwiki (a Wikipedia) for some reason, so this essentially means it is impossible to use both modules at once as of now. I'm not sure merging the suppressing patch is a good idea for that reason (but @Legoktm already merged).

The test failure is mostly useful to prevent new conflicts from being introduced and to surface existing conflicts, now that we know that the issue exists, having the test failing and keeping ProofreadPage out of gate is not useful.

Change 719642 had a related patch set uploaded (by Inductiveload; author: Inductiveload):

[mediawiki/core@master] Revert \"tests: suppress API prefix uniqueness check for 'pi'\"

https://gerrit.wikimedia.org/r/719642

Change 719648 had a related patch set uploaded (by Legoktm; author: Legoktm):

[pywikibot/core@master] Change meta=proofreadinfo to use prpiprop=

https://gerrit.wikimedia.org/r/719648

Change 719582 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] Rename proofreadinfo API's piprop to prpiprop

https://gerrit.wikimedia.org/r/719582

Change 719908 had a related patch set uploaded (by Inductiveload; author: Inductiveload):

[pywikibot/core@master] Update ProofreadPage meta query prefix to prpi

https://gerrit.wikimedia.org/r/719908

Change 719908 abandoned by Inductiveload:

[pywikibot/core@master] Update ProofreadPage meta query prefix to prpi

Reason:

https://gerrit.wikimedia.org/r/c/pywikibot/core/+/719648

https://gerrit.wikimedia.org/r/719908

Change 719642 merged by jenkins-bot:

[mediawiki/core@master] Revert \"tests: suppress API prefix uniqueness check for 'pi'\"

https://gerrit.wikimedia.org/r/719642

This should definitely be included in next tech news issue.

Impact: all clients making API calls like https://en.wikisource.org/w/api.php?action=query&meta=proofreadinfo&piprop=namespaces will have to switch to https://en.wikisource.org/w/api.php?action=query&meta=proofreadinfo&prpiprop=namespaces starting next train (wmf.23).

There is no transition period where both parameter names are recognized, because the default value is "include everything" -- the only impact on clients using an incorrect parameter name is an unrecognized parameter warning (and potentially, response has more info than requested, but never less).

Inductiveload moved this task from Backlog to Done: to deploy/check on the ProofreadPage board.

Change 720862 had a related patch set uploaded (by Jforrester; author: Inductiveload):

[mediawiki/core@wmf/1.37.0-wmf.22] tests: suppress API prefix uniqueness check for 'pi'

https://gerrit.wikimedia.org/r/720862

Change 720862 abandoned by Jforrester:

[mediawiki/core@wmf/1.37.0-wmf.22] tests: suppress API prefix uniqueness check for 'pi'

Reason:

We didn't deploy wmf.21.

https://gerrit.wikimedia.org/r/720862

Change 721066 had a related patch set uploaded (by Jforrester; author: Inductiveload):

[mediawiki/core@wmf/1.37.0-wmf.21] tests: suppress API prefix uniqueness check for 'pi'

https://gerrit.wikimedia.org/r/721066

Change 719648 merged by jenkins-bot:

[pywikibot/core@master] Drop piprop from meta=proofreadinfo API call

https://gerrit.wikimedia.org/r/719648

Change 721066 merged by jenkins-bot:

[mediawiki/core@wmf/1.37.0-wmf.21] tests: suppress API prefix uniqueness check for 'pi'

https://gerrit.wikimedia.org/r/721066

@Urbanecm How could we describe this concisely in Tech News? Ideally, without giving a long URL example within the text.
I imagine something like this:

The API for Wikisource has changed. Any previous uses of piprop=namespaces should now instead be prpiprop=namespaces. [link here]

Please edit that for accuracy, or approve. Thanks!

The [[s:Special:ApiHelp/query+proofreadinfo|meta=proofreadpage API]] has changed. The <code>piprop</code> parameter has been renamed to <code>prpiprop</code>, API users should update their code to avoid unrecognized parameter warnings. Pywikibot users should upgrade to 6.6.0.

Inductiveload claimed this task.

This is now deployed and PWB has also bee updated.