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

Convert runInteraction to async/await #8156

Merged
merged 2 commits into from
Sep 2, 2020
Merged

Convert runInteraction to async/await #8156

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fa3cb7f
Convert runInteraction to async.
clokep Aug 28, 2020
ca31cfe
Add additional typing.
clokep Sep 2, 2020
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/8156.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Convert various parts of the codebase to async/await.
26 changes: 12 additions & 14 deletions synapse/storage/database.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
from typing_extensions import Literal

from twisted.enterprise import adbapi
from twisted.internet import defer

from synapse.api.errors import StoreError
from synapse.config.database import DatabaseConnectionConfig
Expand Down Expand Up @@ -507,8 +506,9 @@ def new_transaction(
self._txn_perf_counters.update(desc, duration)
sql_txn_timer.labels(desc).observe(duration)

@defer.inlineCallbacks
def runInteraction(self, desc: str, func: Callable, *args: Any, **kwargs: Any):
async def runInteraction(
self, desc: str, func: Callable, *args: Any, **kwargs: Any
) -> Any:
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 this can be:

Suggested change
async def runInteraction(
self, desc: str, func: Callable, *args: Any, **kwargs: Any
) -> Any:
async def runInteraction(
self, desc: str, func: "Callable[..., R]", *args: Any, **kwargs: Any
) -> R:

But mypy then complains:

synapse/storage/database.py:550: error: Incompatible return value type (got "R", expected "R") [return-value]

I suspect something isn't getting piped through runWithConnection / new_transaction properly?

Copy link
Member

Choose a reason for hiding this comment

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

So I think the disconnect is happening because mypy doesn't know that the return type of self.new_transaction is the same R as returned by func, due to the indirection of runWithConnection.

The signature runWithConnection(func, *args, **kwargs) means that when we call it with runWithConnection(self.new_transaction, func) mypy only sees us calling self.new_transaction(*args) as the typing of func is lost due to the variadic parameters.

Changing it to runWithConnection(lambda conn: self.new_transaction(conn, func)) works, but it may be simpler to do a result = cast(R, result) instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Woot! Thanks. See ca31cfe for this.

"""Starts a transaction on the database and runs a given function

Arguments:
Expand All @@ -521,7 +521,7 @@ def runInteraction(self, desc: str, func: Callable, *args: Any, **kwargs: Any):
kwargs: named args to pass to `func`

Returns:
Deferred: The result of func
The result of func
"""
after_callbacks = [] # type: List[_CallbackListEntry]
exception_callbacks = [] # type: List[_CallbackListEntry]
Expand All @@ -530,16 +530,14 @@ def runInteraction(self, desc: str, func: Callable, *args: Any, **kwargs: Any):
logger.warning("Starting db txn '%s' from sentinel context", desc)

try:
result = yield defer.ensureDeferred(
self.runWithConnection(
self.new_transaction,
desc,
after_callbacks,
exception_callbacks,
func,
*args,
**kwargs
)
result = await self.runWithConnection(
self.new_transaction,
desc,
after_callbacks,
exception_callbacks,
func,
*args,
**kwargs
)

for after_callback, after_args, after_kwargs in after_callbacks:
Expand Down