Page MenuHomePhabricator

Resolve issues with python-mwapi dependency on LiftWing
Open, Needs TriagePublic

Description

We are using the python-mwapi python package in a lot of our services. This package is currently unmaintained and lives under MediaWiki Utilities. We have discovered a small bug in the package (see here) and some of our planned work is also directly related to this package (e.g. https://phabricator.wikimedia.org/T407843 and https://phabricator.wikimedia.org/T363725), which incentivizes us to look for solutions to the problem.

The current paths forward gathered from Slack discussions are:

  1. Evaluate switching to https://github.com/mwclient/mwclient/ started by an early time volunteer. This one is maintained, has external contributors and is lightweight.
  2. Keep using and maintaining the python-mwapi ourselves. We can either:
  3. Write the new package ourselves :)

This ticket should evaluate the ideas above, we should agree on the path forward and plan the follow-up work before closing it.

Event Timeline

I investigated a little option 1 of switching to mwclient library for inference-services, but I don't think it covers our use-cases well, mostly the async support needed for async KServe services.

Our current usage across multiple services relies on having a shared aiohttp.ClientSession for connection pooling across requests. We use only the raw API GETs, and we also use continuation=True async generators in two places. Outside of those, ores-based revscoring service currently uses a sync mwapi.Session and pins mwapi in requirements as well.

mwclient is fully synchronous built on requests and doesn't currently offer asyncio support at all. Looking into the source code it seems that extending it to async support would also be major work as currently blocking I/O is baked into the API shape (HTTP calls in constructors, sync iterators), which means the public API would have to change to support it. It has an open issue about async support with no movement https://github.com/mwclient/mwclient/issues/320, maintainers suggested that it's for post 1.0 release. Async fork exists, but also has not been maintained for years https://github.com/Wadu436/aiomwclient. It also wouldn't help the planned work: T363725 needs control over aiohttp's per-request redirect behaviour and T407843 needs retries around async calls.

Based on those findings, I'm leaning towards option 2 as it has near-zero migration cost + gives us control over the package. If we agree, the remaining decision is where to host the fork - GitHub vs GitLab.

We discussed the options and want to go with forking mwapi and maintaining it ourselves, hosted on Wikimedia GitLab. Main reasons are close-to-zero migration cost + it unblocks T363725, T407843 and lets us land fixes to mwapi. GitLab seems preferred as it's within our own infrastructure with our own CI, compared to GitHub.
@Dzahn Would you be able to help us create the repo and import the code? Does repos/machine-learning/python-mwapi sound like a good path, or do you prefer a different namespace?

There is also an open question related to name on PyPI - the mwapi library (0.6.1, released 2022-07-26, MIT, author Aaron Halfaker) is not ours to publish to. I think we have 2 options:

  • Ideally, we could add https://pypi.org/user/wikimedia/ as an owner of the existing mwapi project. This would probably need help from Halfak or Yuvi. Additionally, we would need credentials to pypi.org/user/wikimedia.
  • We can publish under a new distribution name e.g. wikimedia-mwapi.

The next steps would be:

  1. Create GitLab repo and import the code with full git history. Keep the MIT licence and the original attribution.
  2. Add CI and a release job that publishes to PyPI
  3. Decide the PyPI name and make the first release (depends on the open question above)
  4. Update requirements in inference-services repo and verify the services work.
  5. Coordinate with other consumers of mwapi package (e.g. knowledge_integrity repo)
  6. Add a README note that this is a Wikimedia-maintained fork.
  7. Do T363725 and T407843 on top of the fork.

Hi @BWojtowicz-WMF sure, no problem.

Just to confirm: You would like to import https://github.com/mediawiki-utilities/python-mwapi right?

It would be simplest if we can use an existing namespace under /repos/. Would it be ok to live under /repos/mediawiki/?

Like /repos/mediawiki/python-mwapi? I do not see an existing /repos/machine-learning so far.

Or maybe /repos/mediawiki/utilities/python-mwapi is better since other mediawiki/utilities could be imported.

Just to confirm: You would like to import https://github.com/mediawiki-utilities/python-mwapi right?

Yes, exactly this one.

It would be simplest if we can use an existing namespace under /repos/. Would it be ok to live under /repos/mediawiki/?
Like /repos/mediawiki/python-mwapi? I do not see an existing /repos/machine-learning so far.
Or maybe /repos/mediawiki/utilities/python-mwapi is better since other mediawiki/utilities could be imported.

I very much like the suggestion of using /repos/mediawiki/utilities/python-mwapi so we could also import other utilities there in the future. The machine learning repo exists in our gitlab already, but I think mediawiki will be a way better place.

Thank you so much for the help @Dzahn !