Can you live with query params "for now" and we move to fixed params in the path if we find it's causing cache problems?

Is your concern that client developers might put the parameters in different order?

Why are we adding from and to as query parameters? It would be a good practice to have parameters in the path as much as possible, cause it's making the paths more predictable - thus better for frontend caching.

Thank you @jijiki. I was a bit confused in the beginning as it didn't work, but sudo systemctl restart php7.2-fpm.service make it work :)

What happens if the rate-limiting service is unavailable or is lagging?

Back in 2014 the following patch has added a limit to the Title::countRevisionsBetween method that we intend to use in the implementation here with a rationale that a query was causing DB timeouts. The query we're going to use is the same, thus I think it has the potential to keep timing out.

What should happen if revision from is newer then revision to?

If you didn't know the first and last revision ID, you'd have to scroll through the whole history to get the first and last ID, at which point you could probably count all the intermediate ones yourself.

Addressed by https://gerrit.wikimedia.org/r/c/mediawiki/core/+/542450 as well.

The check is implemented in https://gerrit.wikimedia.org/r/c/mediawiki/core/+/542450

If we don't go with my proposals in T234940#5580724, we don't need any special queries. There's already Title::countRevisionsBetween method in MW core which provides exactly what's needed.

• Do we envision expanding this endpoint to cover filters like T231590 in the future?
• The /page/history/{title}/counts/edits is a special case of this endpoint as you can get the same information as /revision/<first_page_rev>/intermediatecount/<last_page_revision>. Given that the information provided by the endpoints is so similar that they even intersect, why are the endpoints named so differently and even belong to a different 'namespace'? one is 'page', other is 'revision'.

@jijiki Thank you! However, I will ask you for one more thing in beta - could you also install it on deployment-mediawiki-parsoid10.deployment-prep.eqiad.wmflabs before proceeding to prod? That's where all the requests to rest.php are routed now, so we need it there to test from outside the beta cluster.

The kad library that the DHT rate limiter is based on was forked. Since it worked OK, the fork was never actually updated, and in the time that's passed, the upstream library has moved names 2 times and now looks quite abandoned. So, moving away from using it was inevitable, thus I'm all for the pool counter solution.

csrf token: I don't believe this is necessary if we only support OAuth authentication

The path could be a subresource /page/{title}/wikitext, following the principle that subresources should be used for "larger, more expensive, less-frequently-used properties, or properties with different access rights" (ref), which would seem to describe the wikitext of the page. For HTML delivery we could use /page/{title}/html, but that doesn't seem to be necessary for the CRUD sprint.

I believe that we need to copy the errors from compare endpoint in case of revisions do not belong to the same page -> return 400

@jijiki it appears to be present now :)

We'll keep the endpoints as they are.

I agree with @tstarling

The above patch should include nulls instead of omitting the properties.

Of the three, I prefer a null value. For clients, it's kind of irritating to have properties blink in and out of existence. The empty string is not really a good way to indicate "does not exist".

There's an inconsistency between the count and listing endpoints. that I feel like we could followup on really quick:

In T235146#5564481, @eprodromou wrote:

@BPirkle one last thing (sorry): the original requirements had "delta" and "size" in "characters". What I meant was, "do it exactly like the Web UI does it". Reviewing, the size and delta are in bytes in the Web UI, and I think we should do the same here. (I tried adding multi-byte chars to pages and it looks like it's definitely a difference in bytes).
I don't know if you did it that way in the first place, but I wanted to call it out here.

Bill, if you're making changes, can you return a 200 status code when there are 0 results? I updated the user story to be more specific that the array is 0-20 elements.

In T233179#5560076, @CCicalese_WMF wrote:

Is this task blocking MCR schema migration?

@CCicalese_WMF yeah. Some unexpected complications have been found.

I have to wonder why the IOS team needs all 5 numbers on every page. Then again, I also have to wonder why they need some of these numbers at all.

The api-testing has abstractions and some tests set up for Action API, so we would probably need to invest a bit into setting it up for REST API routes as well before we can write actual tests.

And, should we take the user preferences into account if there's a logged in user, or is it the HTML that anonymous user would see?

So it sounds like the root cause was MassMessageSubmit succeeding but reporting failure (and thus being retried), and that still hasn't been explained or fixed, even though we now have a hack in place to catch those erroneous duplicates, right?

With the completion of the Translate extension, all references has been either removed or gated. Resolving.

Translate extension is done, all usages has been either removed or gated. https://gerrit.wikimedia.org/r/c/mediawiki/core/+/539607 hard deprecates the method.

All access has been either removed or gated. Resolving.

Hi, Natalia. @Anomie pointed out that getting these counts is pretty hard on our infrastructure, so if we get them as separate HTTP calls, each call has lower impact on our servers, and we can spread them out across multiple servers. I realize it means you need to make 5 different calls; is that going to work for you?

html must contain the HTML for the page without any skin or UI, like action=render

reverted

Where a revert is defined as a revision labelled with either Undo or Rollback

I'm open to having this conversation but not to having it block shipping the history endpoint.

In general, the response format doesn't seem to care about the fact we have slots now. I think any new API we're designing should take the existence of revision slots into consideration. Otherwise we're creating more tech debt.

Not reproduced anymore, I guess some purge has been lost. Not much we can do to debug any further now.

1. I think the question came down to the insanely confusing nature of reverse-chronological segmented lists. "next" means "older" and "prev" means "newer". I decided that the advantage of having standard next/prev link relations was offset by having the reverse-chron confusion.

Sorry, I might be late to the party, but I do have some terminology concerns:

This has been relevant during the planning stage of k8s for services, but I guess now we don't need this ticket open anymore.

I'm raising the priority of this yet again cause the fork has completely diverged from the upstream and it's getting to the point when it's dependencies are not supported either, so we really need to switch away from the fork.

Yay! Sorry about the regression.

This should be resolved now. Please test.

https://github.com/wikimedia/change-propagation/pull/331 should fix it.

Ok. I know what's happening, I'm sorry about that. This is a regression of https://gerrit.wikimedia.org/r/c/mediawiki/extensions/MassMessage/+/538742

Not ideal, but I think it's a relatively minor maintenance burden...?

Could you find out why do they need data-parsoid? It's internal information useful for Parsoid. Exposing it via API is deprecated. Data-parsoid was never considered a part of the API, and AFAIK is not stable.

Does this depend on any recent core changes? I was testing with about week old core code.

We're not using eventlogging-eventbus service anymore, it was replaced by eventgate, please create a new task if timeouts keep happening.

All of the above are addressed by the existing patches.

Multiple annotations directly in definition looks better.

The patch above deals with the only occurrence of *_text_id in the flow extension. Neither getRevisionText nor getQueryInfo with a text flag is present.

In T232379#5506991, @sguebo_WMF wrote:

Hey @Pchelolo, do we know wether the patch might be merged soon? Thanks for the work there.