-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Finish adding type hints to the HomeServer #9374
Conversation
if user_device_resync is None: | ||
failures[destination] = { | ||
"status": 503, | ||
"message": "Unable to resync devices", | ||
} | ||
continue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that this method assumes that user_device_resync
will raise if the host cannot be reached, but it actually eats errors and returns None
instead. 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM apart from the one Q
synapse/handlers/set_password.py
Outdated
device_handler = hs.get_device_handler() | ||
assert isinstance(device_handler, DeviceHandler) | ||
self._device_handler = device_handler | ||
self._device_handler = hs.get_device_handler() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So something was trying to construct this on a worker? Can we make it so that that doesn't happen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll look back into it!
This annoying ends up with circular includes, so...need to figure out what to do about that. 😢 |
Deleting this since it is easier to do it in bits and pieces. |
There were a few spots that we didn't have return types on
HomeServer
this adds them! Unfortunately there's a few spots where we return different types, which makes the code rather messy. I'm not 100% sure this is worth it, although it did actually find a couple of discrepancies.