Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Page MenuHomePhabricator

Removal of writeapi from siteinfo output breaks all mwclient-based bots, including stashbot (Server Admin Log)
Open, Needs TriagePublicBUG REPORT

Description

Stashbot now just shows the following error whenever someone tries to log a message (seen in #wikimedia-cloud and #wikimedia-operations):

<stashbot> dcaro: Failed to log message to wiki. Somebody should check the error logs.

stashbot.log
Error writing to wiki
Traceback (most recent call last):
  File "/data/project/stashbot/stashbot/sal.py", line 204, in log
    url = self._write_to_wiki(bang, channel_conf)
  File "/data/project/stashbot/stashbot/sal.py", line 373, in _write_to_wiki
    resp = page.save("\n".join(lines), summary=summary, bot=True)
  File "/data/project/stashbot/venv-k8s-py39/lib/python3.9/site-packages/mwclient/page.py", line 179, in save
    return self.edit(*args, **kwargs)
  File "/data/project/stashbot/venv-k8s-py39/lib/python3.9/site-packages/mwclient/page.py", line 184, in edit
    return self._edit(summary, minor, bot, section, text=text, **kwargs)
  File "/data/project/stashbot/venv-k8s-py39/lib/python3.9/site-packages/mwclient/page.py", line 207, in _edit
    raise mwclient.errors.NoWriteApi(self)
mwclient.errors.NoWriteApi: <Page object 'b'Server Admin Log'' for <Site object 'wikitech.wikimedia.org/w/'>>

Looking at the mwclient source code, I think this is because mwclient expects to see writeapi in the action=query&meta=siteinfo response:

# Extract site info
self.site = meta['query']['general']
self.namespaces = {
    namespace['id']: namespace.get('*', '')
    for namespace in meta['query']['namespaces'].values()
}
self.writeapi = 'writeapi' in self.site

(And if self.writeapi [where self is a Site] is false, then it throws the NoWriteApi error seen in the logs later:)

def _edit(self, summary, minor, bot, section, **kwargs):
    # (snip)
    if not self.site.writeapi:
        raise mwclient.errors.NoWriteApi(self)

With the current train status, that flag is still present on enwiki, but not on Wikitech, because we dropped writeapi flag from siteinfo API as part of T294397: Drop writeapi MediaWiki right. I believe this breaks, or will break (when the train reaches the relevant wiki), all mwclient-based bots which make edits.

Related Objects

Event Timeline

LucasWerkmeister triaged this task as Unbreak Now! priority.Wed, Aug 7, 1:43 PM
LucasWerkmeister created this task.
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
LucasWerkmeister changed the subtype of this task from "Production Error" to "Task".

Change #1060454 had a related patch set uploaded (by Lucas Werkmeister; author: Lucas Werkmeister):

[mediawiki/core@master] Revert "Drop writeapi flag from siteinfo API"

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

GitHub code search (requires login) shows that pywikibot also makes the flag available in its Siteinfo class (code), though it looks like at least within pwb itself nothing actually reads it. Various other search results also look like people just define types for every member returned by the siteinfo API, whether they need it or not; however, AutoWikiBrowser (AWB) also checks writeapi.

however, AutoWikiBrowser (AWB) also checks writeapi.

Eh… looking closer, it seems to check the writeapi right, not the siteinfo response member. So it’s not gonna be directly broken in the same way, I guess. Let’s untag them again.

Change #1060454 had a related patch set uploaded (by Lucas Werkmeister; author: Lucas Werkmeister):

[mediawiki/core@master] Revert "Drop writeapi flag from siteinfo API"

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

My suggestion is to merge and backport this; however, as I’m at Wikimania, I will not be doing the backport myself. Maybe the train conductors can do it?

Mentioned in SAL (#wikimedia-cloud) [2024-08-07T15:41:31Z] <wmbot~bd808@tools-bastion-12> Installed a hacked up version of mwclient from git+https://github.com/bd808/mwclient@2e9cd61d90738fbf9ee64ca2c1766a9095c24699 to work around T371977

As one of the new(ish) maintainers of mwclient - thanks for alerting us to this (see https://github.com/mwclient/mwclient/issues/344). We are currently trying to pull together a 0.11 release of mwclient, but it's taking a bit of time as we're all spare time maintainers. We'll definitely try and resolve this as part of that release.

Besides Stashbot where this was first discovered, this bug is going to break wiki writes done by Tool-phab-ban, Tool-gitlab-account-approval, Tool-schedule-deployment, Striker, and the OpenStack lifecycle hooks that create Nova Resource namespace pages on wikitech.wikimedia.org when we create a new Cloud-VPS project.

Change #1060468 had a related patch set uploaded (by BryanDavis; author: Lucas Werkmeister):

[mediawiki/core@wmf/1.43.0-wmf.17] Revert "Drop writeapi flag from siteinfo API"

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

Change #1060468 had a related patch set uploaded (by BryanDavis; author: Lucas Werkmeister)

I'll deploy the above shortly.

Change #1060454 merged by jenkins-bot:

[mediawiki/core@master] Revert "Drop writeapi flag from siteinfo API"

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

Change #1060468 merged by jenkins-bot:

[mediawiki/core@wmf/1.43.0-wmf.17] Revert "Drop writeapi flag from siteinfo API"

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

Mentioned in SAL (#wikimedia-operations) [2024-08-07T17:27:31Z] <brennen@deploy1003> Started scap sync-world: Backport for [[gerrit:1060468|Revert "Drop writeapi flag from siteinfo API" (T115414 T294397 T371977)]]

Mentioned in SAL (#wikimedia-operations) [2024-08-07T17:29:44Z] <brennen@deploy1003> brennen, bd808: Backport for [[gerrit:1060468|Revert "Drop writeapi flag from siteinfo API" (T115414 T294397 T371977)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-08-07T17:35:37Z] <brennen@deploy1003> Finished scap: Backport for [[gerrit:1060468|Revert "Drop writeapi flag from siteinfo API" (T115414 T294397 T371977)]] (duration: 08m 06s)

Removing as train blocker for .17, leaving open in case of other followup.

the OpenStack lifecycle hooks that create Nova Resource namespace pages on wikitech.wikimedia.org when we create a new Cloud-VPS project.

In "fun" news, the wmfkeystoneauth and Bitu usage of mwclient is via the python3-mwclient Debian package, so it looks like we will probably need to build and host a .deb for the updated library when it is available.

Change #1060468 had a related patch set uploaded (by BryanDavis; author: Lucas Werkmeister)

I'll deploy the above shortly.

Thanks a lot for taking care of it!

LucasWerkmeister lowered the priority of this task from Unbreak Now! to Needs Triage.Wed, Aug 7, 8:17 PM

In "fun" news, the wmfkeystoneauth and Bitu usage of mwclient is via the python3-mwclient Debian package, so it looks like we will probably need to build and host a .deb for the updated library when it is available.

If we’re going to be reinstating the writeapi removal in production soon-ish, I would guess that other users of the Debian package would also want a fixed version of the library so they don’t get an error when talking to Wikimedia wikis? (Easier said than done, of course. And I don’t know how many third-party users the Debian package actually has – I couldn’t find any other packages depending on it, at least, though that might just be me operating apt wrong.)

If we’re going to be reinstating the writeapi removal in production soon-ish, I would guess that other users of the Debian package would also want a fixed version of the library so they don’t get an error when talking to Wikimedia wikis? (Easier said than done, of course. And I don’t know how many third-party users the Debian package actually has – I couldn’t find any other packages depending on it, at least, though that might just be me operating apt wrong.)

https://tracker.debian.org/pkg/mwclient has the package listed as orphaned and points to https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1068659

"I told you so".

I specifically amended https://gerrit.wikimedia.org/r/c/mediawiki/core/+/392542, which removed the "writeapi" feature to not break the stable API for "siteinfo", by leaving this behind as constant and non-deprecated boolean field. — because its a stable API that projects like Pywikibot still read it, as well as https://github.com/mwclient/mwclient, and no doubt countless other parts of the ecosystem. It doesn't seem worth the churn of deprecation, especially since it's fairly low-level. A typical bot would not per-se have a way to "fix" such warning by not reading something because it happens before they get to it.

From an interface perspective, our contract hasn't changed. It's just always true now.

But then the day after https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1058585 removed it in a separate patch :)

however, AutoWikiBrowser (AWB) also checks writeapi.

Eh… looking closer, it seems to check the writeapi right, not the siteinfo response member. So it’s not gonna be directly broken in the same way, I guess. Let’s untag them again.

For whatever it's worth, there's a report of AWB breakage that is consistent timewise with having broken directly or indirectly due to this. cf. e.g. T372017. It's worth testing for specifically if this change is rolled again.

The AWB breakage is specifically: format=xml&action=query with a body &meta=userinfo&uiprop=blockinfo|hasmsg|groups|rights returns a rights element that no longer includes <r>writeapi</r> (English Wikipedia)

bd808 changed the subtype of this task from "Task" to "Bug Report".Thu, Aug 8, 8:36 PM

I specifically amended https://gerrit.wikimedia.org/r/c/mediawiki/core/+/392542, which removed the "writeapi" feature to not break the stable API for "siteinfo", by leaving this behind as constant and non-deprecated boolean field.

That was a little over six years ago. I don't think we should keep around non-functional API fields for decades just because removing them is more effort; that's the definition of tech debt. It adds mental complexity for both users and maintainers of the API.

That said, it was never properly deprecated. The 1.32 release notes included a deprecation notice for $wgEnableWriteAPI but not for the API response field specifically and there is not reason for a bot maintainer to check the MediaWiki config deprecation notes (or make the connection). So that was a mistake, I should have checked instead of relying on the code comment.

projects like Pywikibot still read it

Pywikibot does not use it for anything, nor did anything else that was discoverable via codesearch. (Well, almost. We did miss some use in Wikibase unit tests.) As a rule of thumb, if you want to make sure your code is not affected by code changes, you should get it into codesearch. (Filed T372144, T372145 about that.)

mwclient-side fix is merged and I intend to do a new mwclient release tomorrow, if nothing else comes up, and none of the other maintainers can think of a reason why not.

To the comment on breaking-or-not siteinfo, please note that the AWB break (now fixed) was due to the flag removed from userinfo. That may come to the same thing; I don't know the internals.

In "fun" news, the wmfkeystoneauth and Bitu usage of mwclient is via the python3-mwclient Debian package, so it looks like we will probably need to build and host a .deb for the updated library when it is available.

If we’re going to be reinstating the writeapi removal in production soon-ish, I would guess that other users of the Debian package would also want a fixed version of the library so they don’t get an error when talking to Wikimedia wikis? (Easier said than done, of course. And I don’t know how many third-party users the Debian package actually has – I couldn’t find any other packages depending on it, at least, though that might just be me operating apt wrong.)

Yes. I created https://bugs.debian.org/1078421 to track this on the Debian side and am planning to fix it there.

To the comment on breaking-or-not siteinfo, please note that the AWB break (now fixed) was due to the flag removed from userinfo. That may come to the same thing; I don't know the internals.

AWB was broken by the removal of the writeapi userright, which is indeed available via userinfo. (Some gadgets were also broken by that, as gadgets can depend on userrights.) There's a patch for that in T372017 if needed, but seems like it isn't.

mwclient was broken by the removal of the writeapi field from siteinfo, which a long time ago returned the value of the $wgEnableWriteAPI configuration variable. That patch was reverted. It would be nice to re-revert it eventually, but there is no reason to rush. We can wait for the next Debian release, or whatever else is needed to ensure wide support.

mwclient 0.11.0 is now released with this fixed on that side. It also comes with four years of other changes, so please test carefully before deploying :)