9 merged pull requests · 5 upstream projects

Open source

Fixes in the libraries I build on: Django, redis-py, matplotlib, OpenTelemetry, and Black. The main one here is a connection-pool bug in redis-py's async cluster client: a topology refresh during a transaction sent connections back to the wrong server's pool. It merged after a maintainer review that changed my approach, and that review is most of the story.

Python · asyncio · Redis Cluster · SQLAlchemy · OpenTelemetry semantic conventions · pytest

Read redis-py #4254 →All merged PRs on GitHub Email about this work

redis-py #4254: connections returned to the wrong pool

While reviewing my earlier redis-py PR (#4206, EVALSHA in cluster pipelines), the maintainer found two places where the async cluster client behaved differently from the sync one and filed them as issue #4253. I took the first one.

The problem

A Redis Cluster transaction that uses WATCH has to send every command on the same socket, because the watch lives on that connection. The async TransactionStrategy held on to its connection correctly, but on every command it looked the slot up again and overwrote _transaction_node with whatever node the slot map named right now. If another task refreshed the cluster topology in the meantime, the pipeline ended up holding a socket from node A while believing it belonged to node B. Then reset() released that socket into node B's free queue, so the next caller on B got a connection to A.

WATCH on node A

pipeline holds a socket to A

Topology refresh elsewhere

slot now maps to node B

Before

node := B · socket still A · reset() frees A into B's pool

After

node stays A · reset() frees A into A's pool

My first version

The sync client has an ownership check: if the pool doesn't own the held connection, release it to its real owner and acquire a new one. I ported that to async with a new ClusterNode.owns_connection() and NodesManager.find_connection_owner(), and argued in the review threads that this was sync parity. Automated reviewers flagged that swapping sockets mid-WATCH was risky. I answered that sync did the same thing. A community reviewer approved it.

The review

Petya Slavova, the maintainer, requested changes. In sync, ConnectionPool.owns_connection() only compares process IDs, so in a single process it always returns true and the swap branch never runs. Sync keeps the watching socket. My async check tested real membership, so async would actually switch sockets, run EXEC without the WATCH, and put a connection that was still watching back in the pool. I had fixed the wrong-pool bug by adding a worse one.

I read redis/connection.py to confirm it, said on the PR that my parity argument was wrong, and took the smaller fix from the review.

The fix that merged

Set the node only when a new connection is acquired. Then the node always names the owner of the held connection, and the existing release paths in reset() and _reinitialize_on_error() are correct without changes. The ownership helpers I'd added were deleted.

if not self._transaction_connection:
    node = self._pipe.cluster_client.nodes_manager.get_node_from_slot(
        list(self._pipeline_slots)[0], False
    )
    self._transaction_connection = node.acquire_connection()
    self._transaction_node = node

return self._transaction_node, self._transaction_connection

The regression test seeds two nodes with mock connections, starts a watched transaction on the first, retargets the slot to the second mid-transaction, and asserts that the pipeline keeps the original socket and that reset() returns it to the first node's pool and not the second's. The approval: "This is exactly the change I had in mind." Merged Oct 20, 2026, about two months after I opened it.

Shorter ones

Django #21629: a nine-year-old test runner crash

Django's parallel test runner gives each worker its own clone of the test database. Worker IDs came from a shared counter that only went up, so when multiprocessing replaced a worker that had exited (a crash, an out-of-memory kill), the new one got an ID past the last clone and tried to open a database that was never created. The run died with database "test_db_5" does not exist on Postgres, or unable to open database file on SQLite. The ticket (#27734) had been open since 2017.

Following the approach a maintainer had sketched on the ticket, I turned the clones into a pool of slots. A worker claims a free slot when it starts and records its PID. On each pass of the existing result-polling loop, the parent frees the slots of workers that have exited, so their replacements reuse those clones. If no slot is free, the worker fails with a clear error. I reproduced the crash on Windows by forcing a respawn after every subsuite (maxtasksperchild=1), and a second reviewer reproduced it on macOS.

Review found two races. I took the snapshot of live processes outside the lock, so a replacement could claim a slot in between, and a worker with a recycled PID could end up owning two slots. Both are handled in the version that merged on July 31, 2026. The reviewer opened with "an impressive first PR."

OpenTelemetry #5063: a breaking change, done the project's way

In the metrics tracking issue I pointed out that the SQLAlchemy instrumentation still reported pool usage as the deprecated db.client.connections.usage. A reviewer opened #5060 from that comment, and I took it. I first put the new metric behind OTEL_SEMCONV_STABILITY_OPT_IN, the way span attributes were migrated. The maintainers decided that flag is for spans only, so I reworked it to always emit db.client.connection.count with the current state and pool-name attributes. Because existing dashboards lose the old metric, it shipped with a breaking-change changelog entry, docs, and tests that check the old metric is gone, run against SQLAlchemy 1 and 2.

OpenTelemetry #4827: finishing a stalled fix

A Celery task request carries a hostname (the worker that ran it) and an origin (the process that sent it). The instrumentation aliased origin to hostname, so on Celery 5 the sender's node name overwrote celery.hostname and traces pointed at the wrong worker while the metrics had it right (#3096). An earlier PR (#3097) had the fix but stalled waiting on a changelog entry and a test. I picked it up, linked the original, and added both. A reviewer pointed out that my test set both fields, so it missed the case the bug lives in, and I added an origin-only test. Merged Oct 26, 2026.

matplotlib #32380: a one-line transform fix

An image made with figimage() disappeared if you removed it and added it back with add_artist() (#31730). figimage() never set a transform, and its pixel offsets only drew correctly because an unset transform falls back to identity. add_artist() sees no transform and assigns the figure's, which draws the image in the wrong place. Setting IdentityTransform() in FigureImage.__init__, as a maintainer suggested in the issue, fixes it for both entry points and still lets transform= override. Milestoned for 3.12.

Everything merged

What I took from it

Before I argue that a change "matches the other code path," I read that code path. I lost a review round on redis-py by trusting a method name.
In a shared library the smallest fix that makes existing code correct beats a new abstraction. My redis-py PR lost two helper methods in review and got better for it.
Most of the calendar time is waiting. I keep the branch rebased and CI green, and ping when it has been quiet for a couple of weeks.

Open to new-grad roles

Resume Email