Convert more of the media code to async/await #7873
Conversation
| for provider in self.storage_providers: | ||
| yield provider.store_file(path, file_info) | ||
| await provider.store_file(path, file_info) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I added some checks in af4f079 for this, hopefully they make sense!
that shouldn't be needed. Basically if we make sure that wherever we get a result from |
|
Thanks! :) I'll take another look and see how I can add that isolation in. |
|
I think you could merge this as-is (well, after fixing the issues) and do the rest of StorageProvider as another step. |
I converted the other couple of methods here (the ones that use |
…rove_test_times * 'develop' of github.com:matrix-org/synapse: (148 commits) Add script for finding files with unix line terminators (#7965) Convert the remaining media repo code to async / await. (#7947) Convert a synapse.events to async/await. (#7949) Convert groups and visibility code to async / await. (#7951) Convert push to async/await. (#7948) update changelog 1.18.0rc1 Fix error reporting when using `opentracing.trace` (#7961) Fix typing replication not being handled on master (#7959) Remove hacky error handling for inlineDeferreds. (#7950) Convert tests/rest/admin/test_room.py to unix file endings (#7953) Support oEmbed for media previews. (#7920) Convert state resolution to async/await (#7942) Fix up types and comments that refer to Deferreds. (#7945) Do not convert async functions to Deferreds in the interactive_auth_handler (#7944) Convert more of the media code to async/await (#7873) Return an empty body for OPTIONS requests. (#7886) Downgrade warning on client disconnect to INFO (#7928) Convert presence handler helpers to async/await. (#7939) Update the auth providers to be async. (#7935) ...
* commit 'f88c48f3b': 1.18.0rc1 Fix error reporting when using `opentracing.trace` (#7961) Fix typing replication not being handled on master (#7959) Remove hacky error handling for inlineDeferreds. (#7950) Convert tests/rest/admin/test_room.py to unix file endings (#7953) Support oEmbed for media previews. (#7920) Convert state resolution to async/await (#7942) Fix up types and comments that refer to Deferreds. (#7945) Do not convert async functions to Deferreds in the interactive_auth_handler (#7944) Convert more of the media code to async/await (#7873) Return an empty body for OPTIONS requests. (#7886) Downgrade warning on client disconnect to INFO (#7928) Convert presence handler helpers to async/await. (#7939) Update the auth providers to be async. (#7935) Put a cache on `/state_ids` (#7931)
So this converts most of the remaining media code to async/await. The remaining pieces after this are the
StorageProviderand sub-classes, but I think those need changes to the S3 code at the same time? I'm not really sure about that.I've held off on pushing these changes for a while since the code surrounding them is confusing and I was not super confident. Please review thoroughly (and also consider if any of this will affect the S3 provider or other third-party providers).