Page MenuHomePhabricator

Promote or remove experimental MW core API endpoints
Closed, ResolvedPublic2 Estimated Story Points

Description

There are a number of "experimental" API endpoints in MediaWiki core.

The last substantial change to these endpoints was in December, 2020. (Subsequent git commits since that time were refactoring for compliance with unrelated core improvements). That's unreasonably long for an API endpoint to be "experimental".

I gave this "medium" priority (and considered giving it "high") because we will soon need to create additional "experimental" endpoints and the current situation both sets a bad precedent, and could complicate the new development.

Review the current "experimental" endpoints to see if they have value. If they do, promote to regular endpoints. If they don't, remove them. If any endpoints to be promoted have existing callers, provide a migration/deprecation path measured in weeks or months, rather than years.

Event Timeline

BPirkle triaged this task as Medium priority.Apr 5 2022, 8:28 PM
BPirkle created this task.

Tried a broad query against analytics data, in hopes that none of these were getting any traffic at all:

"USE wmf; SELECT COUNT(*) AS hits FROM webrequest WHERE uri_path LIKE '%coredev%' AND year = 2022 AND month = 3;"
hits
77

Not much traffic, but not zero either. Will figure out details of what is actually getting hit...

For the record, I think it's a bit of a shame to delete all this. But if it's not used, why keep it? We can always bring them back from the depth of the gerrit history. Or they could be moved into an extension, just so people can find them when they turn out to be needed afterall. We could point to that "graveyard" extension in the coreDevelopmentRouts.json file.

It seems that most of those hits from my overly-broad query were normal hits to pages that happened to have the string "coredev" in the name, rather than API calls. Here are actual REST API coredev hits from March:

/w/rest.php/coredev/v0/revision/983392678/html (one occurrence)
/w/rest.php/coredev/v0/page/Main_Page/links/language (once occurrence)

There were none in February or April.

This is a valid way to check for hits, right? Specifically, I ssh'd into stat1005.eqiad.wmnet and executed queries like:

spark2-sql --master yarn --executor-memory 8G --executor-cores 4 --driver-memory 2G --conf spark.dynamicAllocation.maxExecutors=64 -e "USE wmf; SELECT uri_path AS path FROM webrequest WHERE uri_path LIKE '%rest.php/coredev%' AND year = 2022 AND month = 3;" > coredev.out

(I did month = 2 and month = 4 as separate queries)

Based on those results, at least, nothing is using any of the current coredev endpoints, and we should just delete them. I'll get a patch together to that effect.

Daniel said:

For the record, I think it's a bit of a shame to delete all this.

I don't. Deleting code is the best part of the job. :)

I looked a bit deeper, and (as often happens) things got murky.

tl;dr: I now suggest we remove the experimental routes related to contributions counts, but promote the experimental handlers related to revisions.

This endpoint seems worthy of promoting to "official", and there seems to be little reason not to: /coredev/v0/revision/{id}

It uses the same handler class as this non-experimental endpoint from coreRoutes.json: /v1/revision/{id}/bare

Keeping it seems almost obligatory from a REST perspective, and removing it would only remove a three-line case from a switch statement in the handler.

In the process of looking at that, I noticed something a little more troublesome: the "official" endpoint mentioned above returns, as part of its payload, the url of an experimental endpoint.

https://gerrit.wikimedia.org/g/mediawiki/core/+/e2e5cde968c4cc1edc7d6da992bf987ec1a4c257/includes/Rest/Handler/RevisionSourceHandler.php#53

Not sure why we thought this was a good idea, but it is live in production. Try this, and notice the "html_url" field near the bottom:

https://en.wikipedia.org/w/rest.php/v1/revision/1079778472/bare

Based on my analytics data queries, nobody is actually using /revision/{id}/html. But removing it would mean removing a field from the official endpoint, which I'm a little hesitant to do. The RevisionHtmlHandler class (which powers the experimental endpoint) is only 150 lines long, so I suppose removing it doesn't give a great benefit to maintainability. In the process of promoting it, I'd also modify the existing official route to properly reference the promoted endpoint. This would give us a reasonable set of revision-related endpoints, and if we already have some of them in production, there's a good argument that we should have all of them.

I remain of the opinion that we should remove the four experimental endpoints related to contributions counts, along with the associated handler classes and tests. I can't find anywhere they're used or referenced, they're self-contained so their removal doesn't leave anything with missing pieces, and they'd be easy enough to resurrect from git as a group if we ever need to. My understanding of the stable interface policy is that we can simply remove them without any sort of deprecation or warning. Please let me know if I'm incorrect about that.

Change 779911 had a related patch set uploaded (by BPirkle; author: Bill Pirkle):

[mediawiki/core@master] Promote experimental revision rest endpoints to official

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

Change 779917 had a related patch set uploaded (by BPirkle; author: BPirkle):

[mediawiki/core@master] Remove experimental contributions endpoints

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

Change 779911 merged by jenkins-bot:

[mediawiki/core@master] Promote experimental revision rest endpoints to official

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

We are considering whether to remove or promote the experimental contributions endpoints.

T248579: User contributions API indicates that the experimental contributions endpoints were for use by Android. However, I found no traffic to them. Were they for an experiment that has concluded? Or for a feature that has not yet been implemented? Or maybe my search was flawed and they are actually being used?

I'm not sure who to ask. @JTannerWMF ? @MattCleinman ?

Adding @Dbrant - Do you know the history of these? (These pre-date my and Jazmin's time on the team, but thinking Dmitry may know off the top of his head.)

I don't think we ever ended up using these endpoints (and we certainly don't use them today).
I'm guessing this was part of a larger project (having to do with Suggested Edits) that never quite got off the ground, and we simply ended up using existing standard APIs instead.

iOS also doesn't use them or plan to. Seems like they're safe to remove, at least from the standpoint of mobile apps.

Thank you! I'm not sure yet what their disposition will end up being, but it is good to know that we have all our options available without stepping on anyone's toes.

Status update: patch to remove the contributions endpoint remains in-flight, but we have not conclusively determined whether to land that as-is. Other options on the table include moving the endpoints to an extension. or promoting them to "official".

Status update: we are deferring a decision on the contributions experimental endpoints until the new API Platform Product Manager joins and has time to onboard. If they become problematic in the meantime, please let me know.

We decided to go ahead and remove the endpoints. We can always bring them back from Git histroy.

Change 779917 merged by jenkins-bot:

[mediawiki/core@master] Remove experimental contributions endpoints

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

Change #1019273 had a related patch set uploaded (by Tchanders; author: Tchanders):

[mediawiki/core@master] Remove ContributionsLookup service

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

Change #1019273 merged by jenkins-bot:

[mediawiki/core@master] Remove ContributionsLookup service

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