So just hard deprecate the entire method?

Sounds good. Is it possible to use UserIdentity instead of the User?

See https://gerrit.wikimedia.org/r/c/mediawiki/core/+/596502 - we opted for just removing the use of this method entirely. Should probably do the same in other occurrences if they exist

Actually, after a bit of more thinking, I wonder if LogEntry should instead be converted into a value object - only holding the fields with getters/setters and containing no logic, and database operations, and all the stuff that uses the dependency should be done via a separate service, or even a group of services.

Will do; is this then approved from a product point of view?

All patches here were deployed. I've done some logstash searching, and all errors have completely stopped around May 04-05 which corresponds with deployment. Resolving. Please reopen if reappears.

Verified that the error has gone away. Resolving.

@ssastry please correct me if I'm wrong, but Linter extension stores Parsoid lints in MySQL. If I understand it well, the flow is the following: page edit -> parsoid rerender -> parsoid calling MW API to record lint errors -> API is scheduling a job -> errors are stored in the database. So, VE doesn't need to call RESTBase to re-render the lint errors, it can just execute an action API query if working from the client side, or depend on Linter extension and get the errors from linter table directly.

ManualLogEntry implements a LogEntry interface, and has a sibling: DatabaseLogEntry with RCDatabaseLogEntry as a specialized dependent. DatabaseLogEntry is only created via static factory methods, not via direct construction.

We'd still need to figure out how to make eventgate-wikimedia generate static stream config from EventStreamConfig API at runtime. We'd have to have some script that runs during deployment and gets all stream configs that match stream_config_allowed_settings and renders out a static stream-config.yaml file that is used as the stream_config_uri.

The two patches above introduce the new hook parameter and demonstrate it's usage in an extension. I will make patches to the rest of the extensions once the review of the core patch is completed and we're in agreement on the API.

In T175224#6147310, @tstarling wrote:

The ServiceWiring code was introduced in August 2016. Apparently nothing used it until January 2020, when it was used in an experimental REST handler, but that handler will presumably fail in production due to this bug.

Some tests for the new service would be nice.

The patches were merged, but have not been deployed yet. I'm on vacation next week, so to unblock you I'm asking Hugh to deploy restbase with your changes next week

I very much support this proposal.

I was asking @Pchelolo as the person who filed the task, but sure. :-)

I'm not sure what else to do here.

It seems to me like the only thing useful for the purge handler is the timestamp of when the purge event was created by the producer. Anything about what Last-Modified will be or when some cascading chain started seems like things it either can't or shouldn't use, right? Having said that - what will this actually be used for?

Given that the end goal is not just decoupling, but also moving us closer to fixing the problems with the table described by @Krinkle in T146585#4233276 we need to change the API to perhaps eventually disallow NULL user_last_timestamp so that we're able to switch to upsert and remove duplicated rows, so I have deprecated passing null revision into setNewtalk. However, maybe this requires course correction.

Both user_id and user_ip in Postgres now have default values, so this should be fixed. See https://gerrit.wikimedia.org/r/c/mediawiki/core/+/589345

We got a few more:

Is his done? Can this be closed?

We've actually broken purging again, bu fixed now. We are going to finish working on purges soon, so we would stop breaking this.

In T252146#6119587, @JoeWalsh wrote:

@Pchelolo Thanks for the response, that sounds good. For the case that the user edits the Wikidata description for the article and we don't have a new article revision will it work to request the existing article revision or is the polling method required?

You're talking about mobile-html endpoint right? It supports providing a revision ID to require a new revision. On normal views it's better to use URI without a revision ID, so that you have better cache hit ratio, but after en edit supplying revision ID in the request will have a lot of benefits:

Now that the TalkPageNotificationManager was introduced, we can start using it. However, here it will not be a one-to-one replacement for calling methods.

So this means that beta will be up to date as soon as the updater patch merges. We hit a small snag there, but I think this will happen later today. @Reedy you can speed things up be reviewing https://gerrit.wikimedia.org/r/c/mediawiki/core/+/595139 :)

Tested again. The content is ok after edits.

That makes more sense. This one's to blame: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Scribunto/+/589699

Hey @thcipriani yeah... somehow the issue stacked up on this train.

Please give it about 10-20 minutes before verifying, it needs to catch up on the backlog

I've had a similar idea before in T237692 but I like this proposal better.

I'm closing this as a duplicate of T252145 - that approach would be better since not all the representations exist as core MW concepts.

@JoeWalsh should be fixed now. Our sincere apologies, we are going to be done messing with this soon.

I know what's going on. That's me and @hnowlan breaking things. Will fix right away.

My bad, missed it in the review :(

@Pchelolo thanks for updating this documentation - are there specifics about which characters are allowed and which characters need to be encoded?

In T250209#6109408, @JoeWalsh wrote:

In T250209#6109333, @Pchelolo wrote:

Apologies for the delays. It has happened this morning.

Thanks for the update! It looks like I'm still seeing the same issue. I added a title description to this article with this edit and the page is still outdated. It's properly updated on labs. It looks like the summary hasn't been updated either.

Apologies for the delays. It has happened this morning.

The FileRepresentation spec calls for 'size' property. However, sine we're doing the thumbnailg lazily it is impossible to know at the time of returning the response. Switching to non-lazy generation of thumbnails just to know the size is not acceptable from the performance perspective. We will be dropping the 'size' key from the response.

cc @Ottomata ^^ it seems like all these are simple enough to get into our api-request schema.

But I guess you'll need to do a new chart release and deploy that in order for the new config to take effect — is that correct?

RESTBase configuration deployed.

RESTBase was enabled on the new domain: https://vi.wikipedia.beta.wmflabs.org/api/rest_v1/

I don't think this is much of an improvement - with all the trouble of updating all the extension, we could probably do a more proper solution, like ArticleFactory interface with some MW service implementing it. However, I am not sure we're ready to be doing something like this, and in general untrue of the future of the Article class.

All the instances of deprecated calls mentioned in this task have been cleaned up, except AbuseFilter. There it seems like the code is correct, but we've been using a cached serialized variable - thus calling on Article. Let's see if it resurrects next week.

So, one thing that's touching the code around it is: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/EventBus/+/590471 - not very related, just fyi.

Ok, heh.. the fix wasn't actually deployed it seems. It's been merged, but not yet moved into production. I'll ping you when it is.

In T251599#6099745, @daniel wrote:

@Pchelolo you said a while ago you might look into putting a postgres mode into MediaWikiDockerDev. Did you get around to doing that?

Complete stack trace:

It's PageEditStash. See my comment above.

The other associated log actually gives more insight on which lock is making the transaction timeout:

@daniel I don't think this is that lock showing up here. Reading the code, it seems like if that lock was taken, we should've seen a bunch more queries in this transaction, like some deletes, read from the archive table etc.

The content that was already stored will not be regenerated right away after this change. If you want to regenerate all the content, I guess we'd have to truncate mobile-html table, or do a dump or bump the mobile-html version

Looking at the requestId of one of these errors, the following log messages are found: https://logstash.wikimedia.org/goto/3e387a16e01ae0f02e72b413d6691539

Seems like all the mysteries here have been resolved. Additinally, I verified we're purging mobile-html in the same cases when we purge the old mobile-sections, so we should be good.

We do not use enqueue job anymore in the wikimedia job queue.