Replace Twisted with aiohttp and native asyncio#454
Open
sandhose wants to merge 9 commits intoquenting/modern-tooling/rufffrom
Open
Replace Twisted with aiohttp and native asyncio#454sandhose wants to merge 9 commits intoquenting/modern-tooling/rufffrom
sandhose wants to merge 9 commits intoquenting/modern-tooling/rufffrom
Conversation
2 tasks
7ba52a0 to
8955880
Compare
a7c093c to
1bb8aa4
Compare
8955880 to
e5cab6f
Compare
1bb8aa4 to
5f86b8d
Compare
e5cab6f to
c89a35e
Compare
5f86b8d to
6a1be12
Compare
c89a35e to
fcf8ce2
Compare
65d3fa2 to
cb6f287
Compare
5403098 to
d0cea1d
Compare
78aadaa to
f96764e
Compare
d0cea1d to
e0a99f3
Compare
f96764e to
e1c6876
Compare
d7e0207 to
5a1ae0f
Compare
e1c6876 to
8713ae8
Compare
5a1ae0f to
da1e07e
Compare
8713ae8 to
06655a7
Compare
1f402da to
f5c3e91
Compare
06655a7 to
9037622
Compare
f5c3e91 to
2d16192
Compare
8da7963 to
ea0f3c9
Compare
2d16192 to
2f2a872
Compare
2f2a872 to
be6956e
Compare
ea0f3c9 to
9d22a3d
Compare
be6956e to
e6b6eb8
Compare
9d22a3d to
1b905ff
Compare
e6b6eb8 to
a01ee4d
Compare
4a2c52e to
5b6b2c5
Compare
44fd78f to
571943d
Compare
5b6b2c5 to
2e5242f
Compare
571943d to
38323bd
Compare
2e5242f to
59748b5
Compare
Trial used to run tests in a temp directory, but pytest does not. Use tempfile.mkdtemp() so generated cert files don't pollute the project root.
Remove the Twisted dependency entirely, replacing it with: - aiohttp.web for the HTTP server - aiohttp.ClientSession for HTTP clients (GCM, WebPush) - asyncio.Semaphore for concurrency limiting - asyncio.sleep for delays - aioapns native proxy_host/proxy_port for APNS proxy support - pytest + pytest-asyncio + pytest-aiohttp for tests This deletes the custom Twisted proxy agent, CONNECT protocol, event loop wrapper, and TLS context factory — all replaced by aiohttp's built-in proxy support and ssl.SSLContext defaults.
The retry tests (test_api_v1_retry, test_retry_on_5xx) were sleeping for real (70s and 30s respectively) because asyncio.sleep replaced the old twisted_sleep which used a fake reactor clock. Also reduce the concurrency test sleep from 1s to 10ms.
Rename TestCredentials, TestGcmPushkin, and TestPushkin to StubCredentials, StubGcmPushkin, and StubPushkin so pytest doesn't try to collect them as test classes. Also filter the google-auth AuthorizedSession deprecation warning.
Wire pytest's tmp_path through the base TestCase fixture so tests can use it for temporary files. Migrate the GCM service account file to use tmp_path instead of NamedTemporaryFile.
Close aiohttp.ClientSession instances properly on shutdown in GCM and WebPush pushkins. Add Pushkin.close() base method and call it from Sygnal._start()'s finally block.
- Remove mypy.ini sections referencing deleted files (proxyagent_twisted, twisted_test_helpers, asyncio_test_helpers, test_httpproxy_*) - Use CIMultiDict for DummyResponse.headers so .getall() works if tests ever exercise the retry/5xx code path - Wrap individual pushkin.close() calls in try/except during shutdown so one failure doesn't prevent cleanup of remaining pushkins Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Track status_code through early returns and increment PUSHGATEWAY_HTTP_RESPONSES_COUNTER in the finally block for requests that don't reach _handle_dispatch. - Add close() to ApnsPushkin to shut down the aioapns connection pool on shutdown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fb6c10c to
475082f
Compare
59748b5 to
184a6e9
Compare
reivilibre
reviewed
Apr 7, 2026
tests/twisted_test_helpers.py
Outdated
| csr_filename = "server.csr" | ||
| cnf_filename = "server.%i.cnf" % (cert_file_count,) | ||
| cert_filename = "server.%i.crt" % (cert_file_count,) | ||
| tmpdir = tempfile.mkdtemp(prefix="sygnal-test-certs-") |
Contributor
There was a problem hiding this comment.
Something I wound up with when trying to pytestify Synapse:
$ cat tests/conftest.py
import os
import tempfile
from collections.abc import Iterator
import pytest
@pytest.fixture(scope="session", autouse=True)
def _run_tests_in_temporary_working_directory() -> Iterator[None]:
"""Run pytest from a temporary working directory.
The test suite contains many trial-style tests which write temp files relative
to the current working directory. Running from a temporary directory keeps the
repository worktree clean when running under pytest.
"""
old_cwd = os.getcwd()
with tempfile.TemporaryDirectory(prefix="synapse-pytest-") as tempdir:
os.chdir(tempdir)
try:
yield
finally:
os.chdir(old_cwd)I think it might be as elegant as it gets and also cleans up the dir?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resource/Site/NOT_DONE_YET) withaiohttp.webProxyAgent/readBody/FileBodyProducer) withaiohttp.ClientSessionasyncio.run()+aiohttp.web.AppRunnerDeferredSemaphorewithasyncio.Semaphoreproxy_host/proxy_portinstead ofProxyingEventLoopWrappertwisted.trialtopytest+pytest-asyncio+pytest-aiohttpPushkin.close()lifecycle method for proper session cleanup on shutdownApnsPushkin.close()to shut down the aioapns connection poolpushkin.close()in try/exceptPUSHGATEWAY_HTTP_RESPONSES_COUNTERto count 400 responses (early returns)mypy.inisections referencing deleted filesCIMultiDictforDummyResponse.headersso.getall()works correctlyKnown limitations
os.environ["HTTPS_PROXY"]is set as a global side-effect for the Google Auth session proxy. Pre-existing pattern, carried over.asyncio.run()default behavior. May need attention for container deployments.test_overlong_requests_are_rejecteddeleted —client_max_sizeenforces the limit but is not tested.Test plan
uv run pytest tests/— 55 tests pass