Page MenuHomePhabricator

ApiJsonSchema implements ApiBase::getCustomPrinter for no good reason
Closed, ResolvedPublic

Description

Just because the schema is written in JSON isn't a justification for the schema data itself to not be retrievable using other supported API formats.

If you wait for Gerrit change 182858 to be merged, you won't even have to worry about getting indexed tag names right for format=xml.

In fact, you simply need to remove the implemented getCustomPrinter method and test the change.

Event Timeline

Anomie created this task.Mar 3 2015, 10:08 PM
Anomie updated the task description. (Show Details)
Anomie raised the priority of this task from to Low.
Anomie moved this task to Non-core-API stuff on the MediaWiki-API board.
Anomie added a subscriber: Anomie.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 3 2015, 10:08 PM
Nemo_bis set Security to None.
Florian updated the task description. (Show Details)Jan 8 2016, 3:59 PM

Change 264494 had a related patch set uploaded (by Anomie):
Remove getCustomPrinter() from ApiJsonSchema

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

Change 264591 had a related patch set uploaded (by Florianschmidtwelzow):
Remove getCustomPrinter() from ApiJsonSchema

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

Adding @greg and @20after4 because of the comment from @Anomie on the change: https://gerrit.wikimedia.org/r/#/c/264494/

From my perspective, the best solution would be:

  1. Merge the change (https://gerrit.wikimedia.org/r/#/c/264494/) before next Tuesday (before the new wmf.11 branch was created)
  2. Cherry pick the change to wmf.10 (https://gerrit.wikimedia.org/r/#/c/264591/)
  3. While wmf.11 is deployed to group1 wikis at Wednesday, merge and swat deploy the cherry picked wmf.10 change to group2 (wikipedias)

Maybe Greg or 20after4 has a better idea? :)

greg added a comment.Jan 19 2016, 11:33 PM

Well, we missed today's branching, but otherwise that is a sane plan. :)

Change 264591 abandoned by Florianschmidtwelzow:
Remove getCustomPrinter() from ApiJsonSchema

Reason:
we should do this for the next branching point :)

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

Change 265219 had a related patch set uploaded (by Florianschmidtwelzow):
Remove getCustomPrinter() from ApiJsonSchema

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

Well, we missed today's branching, but otherwise that is a sane plan. :)

So, the new plan is:
From my perspective, the best solution would be:

  1. Merge the change (https://gerrit.wikimedia.org/r/#/c/264494/) before next Tuesday (before the new wmf.12 branch was created)
  2. Cherry pick the change to wmf.11 (https://gerrit.wikimedia.org/r/#/c/265219/)
  3. While wmf.12 is deployed to group1 wikis at Wednesday, merge and swat deploy the cherry picked wmf.11 change to group2 (wikipedias)

@greg Do I need to add a special notice to the Deployments page in wikitech, so you (or the deployer) doesn't miss this special change while deploying to group1? :)

greg added a comment.Jan 20 2016, 4:13 PM

Yeah, getting it on the deployments wiki page so it's not missed is important (I'm mostly good about seeing pings in Phab, but not always as quickly as needed, see: this task ;) ).

:P Ok, thanks for the answer! :) I'll edit wikitech:Deployments, here's the diff. I hope that's ok so, if not... it's a wiki, feel free to edit :)

greg added a comment.Jan 25 2016, 9:51 PM

Update on wmf.11 rollout:

We had to rollback to wmf.10 last week (ie: right now all wikis are on wmf.10).

We are going to redo wmf.11 this week (with appropriate backports) following the normal schedule (just maybe an hour earlier for coverage reasons).

In summary: This is delayed another week.

Change 265219 abandoned by Krinkle:
Remove getCustomPrinter() from ApiJsonSchema

Reason:
Unused branch.

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

Framawiki moved this task from Backlog to Doing on the good first bug board.Dec 2 2017, 1:39 PM
Restricted Application added a project: Analytics. · View Herald TranscriptDec 2 2017, 1:39 PM
Restricted Application added a subscriber: TerraCodes. · View Herald Transcript
Restricted Application added a project: Analytics. · View Herald TranscriptDec 4 2017, 5:01 PM
Legoktm added subscribers: fdans, Legoktm.

@fdans This is an issue with the EventLogging MediaWiki extension.

Restricted Application added a project: Analytics. · View Herald TranscriptDec 4 2017, 6:19 PM
fdans added a comment.Dec 11 2017, 5:15 PM

@Anomie this task is pretty old, is this still going on?

The problem seems to be that it has to be very carefully deployed, and the people who can do that forgot about it after unrelated troubles in wmf/1.27.0-wmf.11.

Or the patch at https://gerrit.wikimedia.org/r/#/c/264494/ could be split into two parts: one to add format=json to future-proof the requests, and the second to be deployed a week later that actually removes the custom printer.

Change 264494 merged by jenkins-bot:
[mediawiki/extensions/EventLogging@master] Remove getCustomPrinter() from ApiJsonSchema

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

Change 264494 merged by jenkins-bot:
[mediawiki/extensions/EventLogging@master] Remove getCustomPrinter() from ApiJsonSchema

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

FYI, remember the discussion above (starting at T91454#1939833) about care that needs to be taken when rolling this out.

zeljkofilipin raised the priority of this task from Low to Unbreak Now!.Mar 18 2019, 4:07 PM
zeljkofilipin added a subscriber: zeljkofilipin.

Train blockers are UBN!

Restricted Application added a subscriber: Liuxinyu970226. · View Herald TranscriptMar 18 2019, 4:07 PM

Train blockers are UBN!

Note there's nothing to unbreak here, this is more of a "Don't Break Later!". See T206676#5033394 for details.

To summarise for here: I've merged a change in the master branch. This change should be deployed to all wikis at once.

Riding the train would cause problems between group 1 and group 2 on Wednesday/Thursday.

I'll backport the change to wmf.21 (now on all wikis) before wmf.22 goes out to group 0 on Tuesday.

Krinkle claimed this task.Mar 18 2019, 9:31 PM
Krinkle lowered the priority of this task from Unbreak Now! to High.

Why not just back-port it and SWAT today?

Change 497417 had a related patch set uploaded (by Jforrester; owner: IoannisKydonis):
[mediawiki/extensions/EventLogging@wmf/1.33.0-wmf.21] Remove getCustomPrinter() from ApiJsonSchema

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

Change 497417 merged by jenkins-bot:
[mediawiki/extensions/EventLogging@wmf/1.33.0-wmf.21] Remove getCustomPrinter() from ApiJsonSchema

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

Krinkle closed this task as Resolved.Mar 19 2019, 2:26 AM

@Krinkle the commit is merged in wmf.21, but not deployed? Is there anything I need to do?

Deployed, but not up with the on-wiki SAL; they appear in the SAL tool https://tools.wmflabs.org/sal:

02:12 <krinkle@deploy1001> Synchronized php-1.33.0-wmf.21/extensions/EventLogging/includes/ApiJsonSchema.php: If280a4056a (duration: 00m 48s)
02:11 <krinkle@deploy1001> Synchronized php-1.33.0-wmf.21/extensions/EventLogging/includes/RemoteSchema.php: If280a4056a (duration: 00m 51s)