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

Get synapse/ to pass mypy #11271

Open
callahad opened this issue Nov 8, 2021 · 2 comments
Open

Get synapse/ to pass mypy #11271

callahad opened this issue Nov 8, 2021 · 2 comments
Labels
O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Dev-Wishlist Makes developers' lives better, but doesn't have direct user impact

Comments

@callahad
Copy link
Contributor

callahad commented Nov 8, 2021

Our mypy configuration currently includes an explicit enumeration of files to check. That list covers 409 of the 491 Python files in the synapse/ directory.

We should:

  • Get as many of those to pass as possible
  • Invert the selection criteria so that all files in synapse/ are checked by default, while failing files are explicitly excluded.

As of 4249082, the omitted files are:

  • storage/databases/__init__.py
  • storage/databases/main/cache.py
  • storage/schema/* (25 files)

We should probably start with switching from inclusion to exclusion, and burn down from there. Note that the next release of mypy will have significantly better syntax for multiple exclusions, but it's not out yet, so we'll have to get by with gnarlier regex-based syntax.

(Edited 2022-08-30 by dmr to reflect the status quo.)

@callahad callahad added P2 T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. labels Nov 8, 2021
@DMRobertson DMRobertson added S-Minor Blocks non-critical functionality, workarounds exist. S-Tolerable Minor significance, cosmetic issues, low or no impact to users. Z-Dev-Wishlist Makes developers' lives better, but doesn't have direct user impact O-Uncommon Most users are unlikely to come across this or unexpected workflow and removed P2 S-Tolerable Minor significance, cosmetic issues, low or no impact to users. labels Aug 30, 2022
@DMRobertson
Copy link
Contributor

Things are much better now. The database modules remain a sticking point (related: #11733). It is very hard to untangle the MRO sphagetti into a form that mypy approves of and some extra thought is required.

The schema files are probably the next best thing to tackle. For bonus points, it'd be great if we could define a Protocol which specifies the interface expected of a schema upgrade file and have the mypy plugin enforce that.

@clokep
Copy link
Member

clokep commented Aug 30, 2022

The schema files are probably the next best thing to tackle. For bonus points, it'd be great if we could define a Protocol which specifies the interface expected of a schema upgrade file and have the mypy plugin enforce that.

I had some issues even getting mypy to see those files properly since they're not in modules (there are not __init__ files). So I'm unsure there's much we can do there?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Dev-Wishlist Makes developers' lives better, but doesn't have direct user impact
Projects
None yet
Development

No branches or pull requests

3 participants