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

Convert more of the media code to async/await #7873

Merged
merged 8 commits into from
Jul 24, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
10 changes: 4 additions & 6 deletions synapse/rest/media/v1/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ def respond_404(request):
)


@defer.inlineCallbacks
def respond_with_file(request, media_type, file_path, file_size=None, upload_name=None):
async def respond_with_file(request, media_type, file_path, file_size=None, upload_name=None):
logger.debug("Responding with %r", file_path)

if os.path.isfile(file_path):
Expand All @@ -89,7 +88,7 @@ def respond_with_file(request, media_type, file_path, file_size=None, upload_nam
add_file_headers(request, media_type, file_size, upload_name)

with open(file_path, "rb") as f:
yield make_deferred_yieldable(FileSender().beginFileTransfer(f, request))
await make_deferred_yieldable(FileSender().beginFileTransfer(f, request))

finish_request(request)
else:
Expand Down Expand Up @@ -198,8 +197,7 @@ def _can_encode_filename_as_token(x):
return True


@defer.inlineCallbacks
def respond_with_responder(request, responder, media_type, file_size, upload_name=None):
async def respond_with_responder(request, responder, media_type, file_size, upload_name=None):
"""Responds to the request with given responder. If responder is None then
returns 404.

Expand All @@ -218,7 +216,7 @@ def respond_with_responder(request, responder, media_type, file_size, upload_nam
add_file_headers(request, media_type, file_size, upload_name)
try:
with responder:
yield responder.write_to_consumer(request)
await responder.write_to_consumer(request)
except Exception as e:
# The majority of the time this will be due to the client having gone
# away. Unfortunately, Twisted simply throws a generic exception at us
Expand Down
16 changes: 7 additions & 9 deletions synapse/rest/media/v1/media_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ def __init__(self, hs, local_media_directory, filepaths, storage_providers):
self.filepaths = filepaths
self.storage_providers = storage_providers

@defer.inlineCallbacks
def store_file(self, source, file_info):
async def store_file(self, source, file_info):
clokep marked this conversation as resolved.
Show resolved Hide resolved
"""Write `source` to the on disk media store, and also any other
configured storage providers

Expand All @@ -61,10 +60,10 @@ def store_file(self, source, file_info):

with self.store_into_file(file_info) as (f, fname, finish_cb):
# Write to the main repository
yield defer_to_thread(
await defer_to_thread(
self.hs.get_reactor(), _write_file_synchronously, source, f
)
yield finish_cb()
await finish_cb()

return fname

Expand All @@ -75,7 +74,7 @@ def store_into_file(self, file_info):

Actually yields a 3-tuple (file, fname, finish_cb), where file is a file
like object that can be written to, fname is the absolute path of file
on disk, and finish_cb is a function that returns a Deferred.
on disk, and finish_cb is a function that returns an awaitable.

fname can be used to read the contents from after upload, e.g. to
generate thumbnails.
Expand All @@ -91,7 +90,7 @@ def store_into_file(self, file_info):

with media_storage.store_into_file(info) as (f, fname, finish_cb):
# .. write into f ...
yield finish_cb()
await finish_cb()
"""

path = self._file_info_to_path(file_info)
Expand All @@ -103,10 +102,9 @@ def store_into_file(self, file_info):

finished_called = [False]

@defer.inlineCallbacks
def finish():
async def finish():
for provider in self.storage_providers:
yield provider.store_file(path, file_info)
await provider.store_file(path, file_info)
Copy link
Member

Choose a reason for hiding this comment

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

sadly I don't think we can rely on the storage providers honouring their interface. In particular, see https://github.com/matrix-org/synapse-s3-storage-provider/blob/master/s3_storage_provider.py#L96-L98: reactor.callInThread returns None, and make_deferred_yieldable passes it through.

in other words: better add some safety checks 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 added some checks in af4f079 for this, hopefully they make sense!


finished_called[0] = True

Expand Down