element-hq / element-hq/synapse

Create a mypy plugin for replication class methods

Open
#8,828 0 comments 0 reactions 0 assignees View on GitHub
A-Workers T-Task Z-Help-Wanted z-p3
Dominant language
Python
Stars
4.6k
Forks
600
Avg merge
5d 22h
Merged PRs (30d)
51

Description

This issue has been migrated from [#8828](https://github.com/matrix-org/synapse/issues/8828).

---

Replication servlet classes, such as `ReplicationRemoteJoinRestServlet`, `ReplicationUserDevicesResyncRestServlet` etc. each have `_serialize_payload` and `_handle_request` methods. Traditionally we haven't added typing to these methods as it breaks mypy for two reasons:

* Their signatures differ from [the base methods](https://github.com/matrix-org/synapse/blob/1781bbe319ce24e8e468f0422519dc5823d8d420/synapse/replication/http/_base.py#L110-L134) in `ReplicationEndpoint`.
* Their signatures differ from `send_request`, which is what is [returned by `make_client`](https://github.com/matrix-org/synapse/blob/2e380f0f1802e5310f5a19b5cac314ce742aaecd/synapse/handlers/e2e_keys.py#L60-L62) and what is run when [making a replication request](https://github.com/matrix-org/synapse/blob/2e380f0f1802e5310f5a19b5cac314ce742aaecd/synapse/handlers/e2e_keys.py#L224-L226).

If types are added to any instance of `_serialize_payload` or `_handle_request`, `mypy` will complain about this discrepancy, and thus an `# type: ignore` has typically been put on the method signature in order to avoid this:

https://github.com/matrix-org/synapse/blob/377673db2ac4b4af6a237252dc39cf464cfb3a30/synapse/replication/http/membership.py#L55-L62

A better solution is to write a mypy plugin, much like [we already have for cache functions](https://github.com/matrix-org/synapse/blob/develop/scripts-dev/mypy_synapse_plugin.py#L31) to properly check the types and remove the need to ignore them.

Note that this issue grew out of a discussion here: https://github.com/matrix-org/synapse/pull/8809#issuecomment-733639886

Contributor guide

Open the contributing guide

Research direction

Start with scripts-dev/mypy_synapse_plugin.py and the ReplicationEndpoint base methods in synapse/replication/http/_base.py. Compare those signatures with _serialize_payload, _handle_request, and send_request in the referenced replication classes and handlers. Done means the plugin checks these method types correctly and the relevant # type: ignore annotations are no longer needed.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
tooling
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.