-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
More annotations for synapse.logging, part 1
#10980
Conversation
Not yet passing `no-untyped-defs`. `make_deferred_yieldable` is tricky and needs more thought.
|
|
||
|
|
||
| def preserve_fn(f): | ||
| def preserve_fn(f: F) -> F: |
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.
We might want add a comment noting that the return type of F changes into a Deferred[T] if the original return type was not a Deferred.
I don't think we came to a consensus in #synapse-devs on whether it was okay to use slightly misleading type hints in this kind of situation.
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.
Ahh, I didn't spot that this also happens in run_in_background. Hmm, maybe this needs more thought?
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 think it should be tractable to add this to our mypy plugin to make it correctly convert the return value from awaitable to deferred?
| def run_in_background(f, *args, **kwargs) -> defer.Deferred: | ||
| def run_in_background( | ||
| f: Callable[..., T], *args: Any, **kwargs: Any | ||
| ) -> "defer.Deferred[T]": |
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.
This doesn't work if T is already an awaitable
|
@DMRobertson I think most of this PR was done in #11556. Should we close this for now? |
|
Agreed! |
Goal is to make
synapse.loggingpassno-untyped-defs. This is a first step.Some work also done in #10943 in this area.