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

improve typing annotations in CachedCall #10450

Merged
merged 2 commits into from
Jul 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changelog.d/10450.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Update type annotations to work with forthcoming Twisted 21.7.0 release.
27 changes: 17 additions & 10 deletions synapse/util/caches/cached_call.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import enum
from typing import Awaitable, Callable, Generic, Optional, TypeVar, Union

from twisted.internet.defer import Deferred
Expand All @@ -22,6 +22,10 @@
TV = TypeVar("TV")


class _Sentinel(enum.Enum):
sentinel = object()


class CachedCall(Generic[TV]):
"""A wrapper for asynchronous calls whose results should be shared

Expand Down Expand Up @@ -65,7 +69,7 @@ def __init__(self, f: Callable[[], Awaitable[TV]]):
"""
self._callable: Optional[Callable[[], Awaitable[TV]]] = f
self._deferred: Optional[Deferred] = None
self._result: Union[None, Failure, TV] = None
self._result: Union[_Sentinel, TV, Failure] = _Sentinel.sentinel
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like _result is an Union[_Sentinel, TV, Failure] but its value is an object? Or am I misunderstanding something here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think you're missing something. Or I don't understand the question.

_result starts out as _Sentinel.sentinel, which (via the magic of Enums) is an instance of _Sentinel. It is later set to either a TV or a Failure.


async def get(self) -> TV:
"""Kick off the call if necessary, and return the result"""
Expand All @@ -78,8 +82,9 @@ async def get(self) -> TV:
self._callable = None

# once the deferred completes, store the result. We cannot simply leave the
# result in the deferred, since if it's a Failure, GCing the deferred
# would then log a critical error about unhandled Failures.
# result in the deferred, since `awaiting` a deferred destroys its result.
# (Also, if it's a Failure, GCing the deferred would log a critical error
# about unhandled Failures)
def got_result(r):
self._result = r

Expand All @@ -92,13 +97,15 @@ def got_result(r):
# and any eventual exception may not be reported.

# we can now await the deferred, and once it completes, return the result.
await make_deferred_yieldable(self._deferred)
if isinstance(self._result, _Sentinel):
await make_deferred_yieldable(self._deferred)
assert not isinstance(self._result, _Sentinel)

if isinstance(self._result, Failure):
self._result.raiseException()
raise AssertionError("unexpected return from Failure.raiseException")

# I *think* this is the easiest way to correctly raise a Failure without having
# to gut-wrench into the implementation of Deferred.
d = Deferred()
d.callback(self._result)
return await d
return self._result


class RetryOnExceptionCachedCall(Generic[TV]):
Expand Down