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

Add missing type hints to synapse.logging.context #11556

Merged
merged 17 commits into from
Dec 14, 2021
Merged

Add missing type hints to synapse.logging.context #11556

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
1afcfd7
Add `defer.gatherResults` wrapper for heterogenous lists of `Deferred`s
Dec 10, 2021
d6dadd9
Use `gather_results` where appropriate
Dec 10, 2021
2f19d0c
Add type hints for `make_deferred_yieldable`
Dec 10, 2021
262e91d
Fix fallout from `make_deferred_yieldable` type hints
Dec 10, 2021
40a48a0
Correct txredisapi stubs
Dec 10, 2021
498bcd7
Add type hints for `run_in_background`
Dec 10, 2021
6d28491
Fix fallout from `run_in_background` type hints
Dec 10, 2021
a4eaf28
Add type hints for `defer_to_thread` and `defer_to_threadpool`
Dec 10, 2021
3c8e9d5
Add type hints for `preserve_fn`
Dec 10, 2021
c305227
Fix fallout from `preserve_fn` type hints
Dec 10, 2021
d32d27b
Add newsfile
Dec 10, 2021
38c7390
Fix cyclic import
Dec 10, 2021
4768efc
Add missing type hints
Dec 10, 2021
5cbf3c3
Reword comment on assert
Dec 10, 2021
1886750
Simplify `make_deferred_yieldable` to only support `Deferred`s
Dec 10, 2021
3f9a04b
Remove tests for now unsupported `make_deferred_yieldable` features
Dec 10, 2021
6980f1e
Merge branch 'develop' into squah/type_hint_synapse_logging_context
squahtx Dec 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion synapse/logging/context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,10 @@ def run_in_background(f, *args, **kwargs) -> defer.Deferred:
return res


def make_deferred_yieldable(deferred):
T = TypeVar("T")


def make_deferred_yieldable(deferred: "defer.Deferred[T]") -> "defer.Deferred[T]":
"""Given a deferred (or coroutine), make it follow the Synapse logcontext
rules:
Copy link
Contributor

Choose a reason for hiding this comment

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

Judging by the doctstring I'd've thought the annotation ought to be Union["defer.Deferred[T]", Awaitable[T]].

This might have cropped up in #10980 --- I remember something like this being painful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, I'd entirely forgotten that there was a previous PR that tried to type hint these. I didn't mean to step on your toes!

Yes, the docstring contradicts the type hint. I wasn't sure what to do here. I can't find anywhere where we feed a coroutine to this function and I would remove the coroutine support, but it feels risky?

I could change it to accept an Awaitable, then I'd need another assert in the implementation, since we can't really handle non-coroutine, non-deferred awaitables.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not to worry---I had let that one rot a little and I don't think I had as robust an understanding of what's going on then as you do now!

Maybe we can remove (or coroutine) from the docstring, if we're convinced that this always is given a Deferred.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've elected to remove the coroutine support, since mypy thinks it is unused, and I can't find any usage with coroutines manually.


Expand Down