This repository was archived by the owner on Apr 26, 2024. It is now read-only.
Finish adding type hints to the HomeServer#9374
Closed
Conversation
clokep
commented
Feb 10, 2021
Comment on lines
+241
to
+246
| if user_device_resync is None: | ||
| failures[destination] = { | ||
| "status": 503, | ||
| "message": "Unable to resync devices", | ||
| } | ||
| continue |
Member
Author
There was a problem hiding this comment.
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. 😢
Member
erikjohnston
left a comment
There was a problem hiding this comment.
LGTM apart from the one Q
| device_handler = hs.get_device_handler() | ||
| assert isinstance(device_handler, DeviceHandler) | ||
| self._device_handler = device_handler | ||
| self._device_handler = hs.get_device_handler() |
Member
There was a problem hiding this comment.
So something was trying to construct this on a worker? Can we make it so that that doesn't happen?
Member
Author
|
This annoying ends up with circular includes, so...need to figure out what to do about that. 😢 |
This was referenced Mar 16, 2021
Member
Author
|
Deleting this since it is easier to do it in bits and pieces. |
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
There were a few spots that we didn't have return types on
HomeServerthis 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.