Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSC2246: Asynchronous media uploads #2246

Draft
wants to merge 5 commits into
base: master
from

Conversation

@tulir
Copy link

commented Aug 24, 2019

Rendered

Proposal for asynchronous media uploads
Signed-off-by: Tulir Asokan <tulir@maunium.net>
@ara4n

This comment has been minimized.

Copy link
Member

commented Aug 24, 2019

sounds excellent to me; would love to unblock streaming transfers at last.
need to consider risk of DoS by creating lots of IDs, and define the json if any handed to the /create (for instance, could be useful in future for defining whether a give mxc url should be locked behind auth of some kind)

Add security consideration and mention possible /create request body
Signed-off-by: Tulir Asokan <tulir@maunium.net>

@turt2live turt2live added the proposal label Aug 24, 2019

@turt2live turt2live self-requested a review Aug 26, 2019

@turt2live
Copy link
Member

left a comment

also generally looking in the right direction - I've left some early comments which are hopefully helpful.

proposals/2246-asynchronous-uploads.md Outdated Show resolved Hide resolved
proposals/2246-asynchronous-uploads.md Outdated Show resolved Hide resolved
proposals/2246-asynchronous-uploads.md Outdated Show resolved Hide resolved
proposals/2246-asynchronous-uploads.md Outdated Show resolved Hide resolved
proposals/2246-asynchronous-uploads.md Outdated Show resolved Hide resolved
tulir and others added 2 commits Aug 26, 2019
Change error code for existing media PUT
Co-Authored-By: Travis Ralston <travpc@gmail.com>
@bmarty

This comment has been minimized.

Copy link

commented Aug 27, 2019

The idea is then to send a media event, after the new /create endpoint, and before sending the media itself; isn't it? We have to consider the failure of the upload: it can lead to media event with eternal not found media.
Also the client should have a way to know that the media is not yet available to the server for a better UI, by displaying a waiting message (ex: Bob is sending an image) for instance?


#### Behavior change in `/download`
When another client tries to download media that has not yet been uploaded, the
content repository should stall the request until it is uploaded. Optionally,

This comment has been minimized.

Copy link
@manuroe

manuroe Aug 28, 2019

We cannot block the request more than timeouts. If the media is not yet available (or not streamable), could we send an error with a retry_after_ms value providing an ETA based on upload speed for example?

This comment has been minimized.

Copy link
@tulir

tulir Aug 29, 2019

Author

Yeah, I think some query param in /download to return an error instead of stalling is needed so clients can do what suits them best. Stalling is probably good as the default since it's backwards-compatible for clients as long as it doesn't time out.

That error could also be used to show some kind of "Bob is sending an image" like @bmarty suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.