Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Use mypy 1.0 #15052

Merged
merged 19 commits into from
Feb 16, 2023
Merged

Use mypy 1.0 #15052

merged 19 commits into from
Feb 16, 2023

Conversation

DMRobertson
Copy link
Contributor

See individual commits for details.

Related: Shoobx/mypy-zope#89

David Robertson added 11 commits February 10, 2023 14:11
Update mypy and mypy-zope
325daae
Suppress false positives from modules not implementing Protocols
408935c 
Not sure why mypy is flagging these up now? Could be a mypy bug

```
synapse/__init__.py:67: error: Argument 1 to "set_json_library" has incompatible type Module; expected "JsonLibrary"  [arg-type]
synapse/__init__.py:67: note: Following member(s) of "Module json" have conflicts:
synapse/__init__.py:67: note:     JSONEncoder: expected "Encoder", got "Type[JSONEncoder]"

synapse/storage/engines/postgres.py:38: error: Argument 1 to "__init__" of "BaseDatabaseEngine" has incompatible type Module; expected "DBAPI2Module"  [arg-type]
synapse/storage/engines/postgres.py:38: note: Following member(s) of "Module psycopg2" have conflicts:
synapse/storage/engines/postgres.py:38: note:     DataError: expected "Type[Exception]", got "Type[DataError]"
synapse/storage/engines/postgres.py:38: note:     DatabaseError: expected "Type[Exception]", got "Type[DatabaseError]"
synapse/storage/engines/postgres.py:38: note:     <9 more conflict(s) not shown>
synapse/__init__.py:67: error: Argument 1 to "set_json_library" has incompatible type Module; expected "JsonLibrary"  [arg-type]

synapse/storage/engines/sqlite.py:29: error: Argument 1 to "__init__" of "BaseDatabaseEngine" has incompatible type Module; expected "DBAPI2Module"  [arg-type]
synapse/storage/engines/sqlite.py:29: note: Following member(s) of "Module sqlite3" have conflicts:
synapse/storage/engines/sqlite.py:29: note:     DataError: expected "Type[Exception]", got "Type[DataError]"
synapse/storage/engines/sqlite.py:29: note:     DatabaseError: expected "Type[Exception]", got "Type[DatabaseError]"
synapse/storage/engines/sqlite.py:29: note:     <9 more conflict(s) not shown>
```
Remove unused ignores
d584aeb 
These used to suppress

```
synapse/storage/engines/__init__.py:28: error: "__new__" must return a
class instance (got "NoReturn")  [misc]
```

and

```
synapse/http/matrixfederationclient.py:1270: error: "BaseException" has no attribute "reasons"  [attr-defined]
```

(note that we check `hasattr(e, "reasons")` above)
Avoid empty body warnings
07d96ca 
E.g.

```
tests/handlers/test_register.py:58: error: Missing return statement  [empty-body]
tests/handlers/test_register.py:108: error: Missing return statement  [empty-body]
```
Suppress false positive about JaegerConfig
1f00d57 
Complaint was

```
synapse/logging/opentracing.py:450: error: Function "Type[Config]" could always be true in boolean context  [truthy-function]
```
Fix not calling is_state()
57861a8 
Oops!

```
tests/rest/client/test_third_party_rules.py:428: error: Function "Callable[[], bool]" could always be true in boolean context  [truthy-function]
```
Suppress false positives from ParamSpecs
9d1d8a6 
````
synapse/logging/opentracing.py:971: error: Argument 2 to "_custom_sync_async_decorator" has incompatible type "Callable[[Arg(Callable[P, R], 'func'), **P], _GeneratorContextManager[None]]"; expected "Callable[[Callable[P, R], **P], _GeneratorContextManager[None]]"  [arg-type]
synapse/logging/opentracing.py:1017: error: Argument 2 to "_custom_sync_async_decorator" has incompatible type "Callable[[Arg(Callable[P, R], 'func'), **P], _GeneratorContextManager[None]]"; expected "Callable[[Callable[P, R], **P], _GeneratorContextManager[None]]"  [arg-type]
````
Drive-by improvement to wrapping_logic annotation
b63789a
Workaround false "unreachable" positives
212897e 
See Shoobx/mypy-zope#91

```
tests/http/test_proxyagent.py:626: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:762: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:826: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:838: error: Statement is unreachable  [unreachable]
tests/http/test_proxyagent.py:845: error: Statement is unreachable  [unreachable]
tests/http/federation/test_matrix_federation_agent.py:151: error: Statement is unreachable  [unreachable]
tests/http/federation/test_matrix_federation_agent.py:452: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:60: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:93: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:127: error: Statement is unreachable  [unreachable]
tests/logging/test_remote_handler.py:152: error: Statement is unreachable  [unreachable]
```
Changelog
a7c2bc3
Linter fixes
852b3fc
@DMRobertson DMRobertson marked this pull request as ready for review February 10, 2023 17:43
@DMRobertson DMRobertson requested a review from a team as a code owner February 10, 2023 17:43
@DMRobertson
Copy link
Contributor Author

As an alternative to 212897e we could also disable --warn-unreachable for the affected files. 🤷

clokep
clokep reviewed Feb 13, 2023
View reviewed changes
Copy link
Member

@clokep clokep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fairly reasonable overall. Seems like mypy has two regressions (or maybe we were just getting away with Any?)

poetry.lock Show resolved Hide resolved
synapse/__init__.py Outdated Show resolved Hide resolved
synapse/streams/__init__.py Show resolved Hide resolved
synapse/handlers/ui_auth/checkers.py Show resolved Hide resolved
synapse/rest/media/v1/_base.py Show resolved Hide resolved
synapse/logging/opentracing.py Show resolved Hide resolved
David Robertson added 3 commits February 15, 2023 14:23
Make UserInteractiveAuthChecker an ABC
8395cc8 
I had to add two new linter ignores here, but it did detect an
incomplete subclass in the tests.
Make Responder an ABC
f351967
Mark EventSource as an ABC
29dd442
@DMRobertson DMRobertson requested a review from clokep February 15, 2023 14:37
DMRobertson pushed a commit to matrix-org/python-canonicaljson that referenced this pull request Feb 15, 2023
Use a property to define Jsonlibrary
20a7e94 
This is suggested/recommended at
python/mypy#6002

Discovered as part of matrix-org/synapse#15052
@DMRobertson DMRobertson mentioned this pull request Feb 15, 2023
Merged
DMRobertson added a commit to matrix-org/python-canonicaljson that referenced this pull request Feb 15, 2023
@DMRobertson
Use a property to define JsonLibrary (#57)
ebab5ec 
* Use a property to define `Jsonlibrary`

This is suggested/recommended at
python/mypy#6002

Discovered as part of matrix-org/synapse#15052

* Okay fine, coverage

* Fix the JsonLibrary protocol

This is fiddlier than I expected.

* Test under mypy 1.0

* Include mypy in default `tox` jobs
@DMRobertson
Copy link
Contributor Author

I've updated this (and force-pushed in a few places, sorry). Will try to see if I can make any progress on #15052 (comment) .

Improve comments in opentracing
14db7d4 
I tried to workaround the ignores but found it too much trouble.

I think the corresponding issue is
python/mypy#12909. The mypy repo has a PR
claiming to fix this (python/mypy#14677) which
might mean this gets resolved soon?
@DMRobertson
Copy link
Contributor Author

Will try to see if I can make any progress on #15052 (comment) .

I gave this a go but couldn't make any progress. I did improve some comments very slightly.

clokep
clokep reviewed Feb 16, 2023
View reviewed changes
poetry.lock Show resolved Hide resolved
clokep
clokep reviewed Feb 16, 2023
View reviewed changes
synapse/handlers/auth.py Outdated
Comment on lines 202 to 205
# mypy sees that the expression `auth_checker_class(hs)` is an instance of
# `UserInteractiveAuthChecker`, but cannot see that it is an instance of
# a non-abstract subclass.
inst = auth_checker_class(hs) # type: ignore[abstract]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to make it think it is concrete? This seems like using any sort of abstractmethod, etc. would...just not work with mypy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a way---thanks for pressing me on this.

Without any annotation, INTERACTIVE_AUTH_CHECKERS is inferred to have type List[Callable[[HomeServer], UserInteractiveAuthChecker]].

Making it List[Type[UserInteractiveAuthChecker]] seems to do the right thing (though I used Sequence because I don't want to suggest it the list of checkers should be mutable).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense -- we're instantiating them here so it should definitely be Type[UserInteractiveAuthChecker]. It seems we had made this mistake quite a bit in the early days...

@DMRobertson DMRobertson requested a review from clokep February 16, 2023 15:33
@DMRobertson DMRobertson enabled auto-merge (squash) February 16, 2023 15:35
@DMRobertson DMRobertson merged commit ffc2ee5 into develop Feb 16, 2023
@DMRobertson DMRobertson deleted the dmr/typing/mypy-1.0 branch February 16, 2023 16:09
@DMRobertson DMRobertson mentioned this pull request Feb 16, 2023
Merged
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Mar 5, 2023
@Midar
Update chat/matrix-synapse to 1.78.0
4fc59f4 
Synapse 1.78.0 (2023-02-28)
===========================

Bugfixes
--------

- Fix a bug introduced in Synapse 1.76 where 5s delays would occasionally occur in deployments using workers. ([\#15150](matrix-org/synapse#15150))


Synapse 1.78.0rc1 (2023-02-21)
==============================

Features
--------

- Implement the experimental `exact_event_match` push rule condition from [MSC3758](matrix-org/matrix-spec-proposals#3758). ([\#14964](matrix-org/synapse#14964))
- Add account data to the command line [user data export tool](https://matrix-org.github.io/synapse/v1.78/usage/administration/admin_faq.html#how-can-i-export-user-data). ([\#14969](matrix-org/synapse#14969))
- Implement [MSC3873](matrix-org/matrix-spec-proposals#3873) to disambiguate push rule keys with dots in them. ([\#15004](matrix-org/synapse#15004))
- Allow Synapse to use a specific Redis [logical database](https://redis.io/commands/select/) in worker-mode deployments. ([\#15034](matrix-org/synapse#15034))
- Tag opentracing spans for federation requests with the name of the worker serving the request. ([\#15042](matrix-org/synapse#15042))
- Implement the experimental `exact_event_property_contains` push rule condition from [MSC3966](matrix-org/matrix-spec-proposals#3966). ([\#15045](matrix-org/synapse#15045))
- Remove spurious `dont_notify` action from the defaults for the `.m.rule.reaction` pushrule. ([\#15073](matrix-org/synapse#15073))
- Update the error code returned when user sends a duplicate annotation. ([\#15075](matrix-org/synapse#15075))


Bugfixes
--------

- Prevent clients from reporting nonexistent events. ([\#13779](matrix-org/synapse#13779))
- Return spec-compliant JSON errors when unknown endpoints are requested. ([\#14605](matrix-org/synapse#14605))
- Fix a long-standing bug where the room aliases returned could be corrupted. ([\#15038](matrix-org/synapse#15038))
- Fix a bug introduced in Synapse 1.76.0 where partially-joined rooms could not be deleted using the [purge room API](https://matrix-org.github.io/synapse/latest/admin_api/rooms.html#delete-room-api). ([\#15068](matrix-org/synapse#15068))
- Fix a long-standing bug where federated joins would fail if the first server in the list of servers to try is not in the room. ([\#15074](matrix-org/synapse#15074))
- Fix a bug introduced in Synapse v1.74.0 where searching with colons when using ICU for search term tokenisation would fail with an error. ([\#15079](matrix-org/synapse#15079))
- Reduce the likelihood of a rare race condition where rejoining a restricted room over federation would fail. ([\#15080](matrix-org/synapse#15080))
- Fix a bug introduced in Synapse 1.76 where workers would fail to start if the `health` listener was configured. ([\#15096](matrix-org/synapse#15096))
- Fix a bug introduced in Synapse 1.75 where the [portdb script](https://matrix-org.github.io/synapse/release-v1.78/postgres.html#porting-from-sqlite) would fail to run after a room had been faster-joined. ([\#15108](matrix-org/synapse#15108))


Improved Documentation
----------------------

- Document how to start Synapse with Poetry. Contributed by @thezaidbintariq. ([\#14892](matrix-org/synapse#14892), [\#15022](matrix-org/synapse#15022))
- Update delegation documentation to clarify that SRV DNS delegation does not eliminate all needs to serve files from .well-known locations. Contributed by @williamkray. ([\#14959](matrix-org/synapse#14959))
- Fix a mistake in registration_shared_secret_path docs. ([\#15078](matrix-org/synapse#15078))
- Refer to a more recent blog post on the [Database Maintenance Tools](https://matrix-org.github.io/synapse/latest/usage/administration/database_maintenance_tools.html) page. Contributed by @jahway603. ([\#15083](matrix-org/synapse#15083))


Internal Changes
----------------

- Re-type hint some collections as read-only. ([\#13755](matrix-org/synapse#13755))
- Faster joins: don't stall when another user joins during a partial-state room resync. ([\#14606](matrix-org/synapse#14606))
- Add a class `UnpersistedEventContext` to allow for the batching up of storing state groups. ([\#14675](matrix-org/synapse#14675))
- Add a check to ensure that locked dependencies have source distributions available. ([\#14742](matrix-org/synapse#14742))
- Tweak comment on `_is_local_room_accessible` as part of room visibility in `/hierarchy` to clarify the condition for a room being visible. ([\#14834](matrix-org/synapse#14834))
- Prevent `WARNING: there is already a transaction in progress` lines appearing in PostgreSQL's logs on some occasions. ([\#14840](matrix-org/synapse#14840))
- Use `StrCollection` to avoid potential bugs with `Collection[str]`. ([\#14929](matrix-org/synapse#14929))
- Improve performance of `/sync` in a few situations. ([\#14973](matrix-org/synapse#14973))
- Limit concurrent event creation for a room to avoid state resolution when sending bursts of events to a local room. ([\#14977](matrix-org/synapse#14977))
- Skip calculating unread push actions in /sync when enable_push is false. ([\#14980](matrix-org/synapse#14980))
- Add a schema dump symlinks inside `contrib`, to make it easier for IDEs to interrogate Synapse's database schema. ([\#14982](matrix-org/synapse#14982))
- Improve type hints. ([\#15008](matrix-org/synapse#15008), [\#15026](matrix-org/synapse#15026), [\#15027](matrix-org/synapse#15027), [\#15028](matrix-org/synapse#15028), [\#15031](matrix-org/synapse#15031), [\#15035](matrix-org/synapse#15035), [\#15052](matrix-org/synapse#15052), [\#15072](matrix-org/synapse#15072), [\#15084](matrix-org/synapse#15084))
- Update [MSC3952](matrix-org/matrix-spec-proposals#3952) support based on changes to the MSC. ([\#15037](matrix-org/synapse#15037))
- Avoid mutating a cached value in `get_user_devices_from_cache`. ([\#15040](matrix-org/synapse#15040))
- Fix a rare exception in logs on start up. ([\#15041](matrix-org/synapse#15041))
- Update pyo3-log to v0.8.1. ([\#15043](matrix-org/synapse#15043))
- Avoid mutating cached values in `_generate_sync_entry_for_account_data`. ([\#15047](matrix-org/synapse#15047))
- Refactor arguments of `try_unbind_threepid` and `_try_unbind_threepid_with_id_server` to not use dictionaries. ([\#15053](matrix-org/synapse#15053))
- Merge debug logging from the hotfixes branch. ([\#15054](matrix-org/synapse#15054))
- Faster joins: omit device list updates originating from partial state rooms in /sync responses without lazy loading of members enabled. ([\#15069](matrix-org/synapse#15069))
- Fix clashing database transaction name. ([\#15070](matrix-org/synapse#15070))
- Upper-bound frozendict dependency. This works around us being unable to test installing our wheels against Python 3.11 in CI. ([\#15114](matrix-org/synapse#15114))
- Tweak logging for when a worker waits for its view of a replication stream to catch up. ([\#15120](matrix-org/synapse#15120))

<details><summary>Locked dependency updates</summary>

- Bump bleach from 5.0.1 to 6.0.0. ([\#15059](matrix-org/synapse#15059))
- Bump cryptography from 38.0.4 to 39.0.1. ([\#15020](matrix-org/synapse#15020))
- Bump ruff version from 0.0.230 to 0.0.237. ([\#15033](matrix-org/synapse#15033))
- Bump dtolnay/rust-toolchain from 9cd00a88a73addc8617065438eff914dd08d0955 to 25dc93b901a87e864900a8aec6c12e9aa794c0c3. ([\#15060](matrix-org/synapse#15060))
- Bump systemd-python from 234 to 235. ([\#15061](matrix-org/synapse#15061))
- Bump serde_json from 1.0.92 to 1.0.93. ([\#15062](matrix-org/synapse#15062))
- Bump types-requests from 2.28.11.8 to 2.28.11.12. ([\#15063](matrix-org/synapse#15063))
- Bump types-pillow from 9.4.0.5 to 9.4.0.10. ([\#15064](matrix-org/synapse#15064))
- Bump sentry-sdk from 1.13.0 to 1.15.0. ([\#15065](matrix-org/synapse#15065))
- Bump types-jsonschema from 4.17.0.3 to 4.17.0.5. ([\#15099](matrix-org/synapse#15099))
- Bump types-bleach from 5.0.3.1 to 6.0.0.0. ([\#15100](matrix-org/synapse#15100))
- Bump dtolnay/rust-toolchain from 25dc93b901a87e864900a8aec6c12e9aa794c0c3 to e12eda571dc9a5ee5d58eecf4738ec291c66f295. ([\#15101](matrix-org/synapse#15101))
- Bump dawidd6/action-download-artifact from 2.24.3 to 2.25.0. ([\#15102](matrix-org/synapse#15102))
- Bump types-pillow from 9.4.0.10 to 9.4.0.13. ([\#15104](matrix-org/synapse#15104))
- Bump types-setuptools from 67.1.0.0 to 67.3.0.1. ([\#15105](matrix-org/synapse#15105))


</details>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@clokep clokep clokep approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@DMRobertson @clokep