-
Notifications
You must be signed in to change notification settings - Fork 158
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
Add TUS creation-with-upload support #3436
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
@jfd this is not working and returns a 400 error: cs3org/reva#710 Also I noticed that the tus-js-client is not sending the header
|
Upload works with this Reva PR: cs3org/reva#725 Now I found out that tus-js-client is not using a fingerprint for uploads that use "creation-with-upload", which means those will not be resumable if the initial POST failed. It will not be resumable because the client has no way of keeping track of this upload so it will not know where to send a HEAD request if the server already had data. In the light of this, I think we should not merge this PR until the library is fixed to start tracking uploads earlier. The doc does say that this feature is currently experimental. Now thinking, the library would not be able to track the download before it gets the "Location" header from the response. This means that the first POST MUST go through. I'm not sure if the "100-Continue", once implemented, would be able to return that header earlier before the payload is sent. A workaround for this would be to purposefully send a smaller chunk for the initial upload to increase the chance of it going through and quickly get access to the "Location" header. @butonic thoughts ? |
As far as I could read, we cannot send additional headers from the server when sending the "100-continue" status code. So the only way would be to decide to send a smaller chunk first to avoid wasting a request and then continuing with the rest in a subsequent request. |
Whenever the server advertised the capability for creation-with-upload, use it automatically for chunked uploads. It is possible to override this and disable it by setting uploadChunkDirect=false in config.json
707475d
to
f0b0c58
Compare
Vincent Petry seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Version 2 of the |
Thanks for the ideas. We'll have a look at the new tus-js-client 2.3.0 soonish. Added this PR for reference in this ticket, so that we can give it another try after the upgrade. |
Description
Whenever the server advertised the capability for creation-with-upload,
use it automatically for chunked uploads.
It is possible to override this and disable it by setting
uploadChunkDirect=false in config.json
Related Issue
None
Motivation and Context
How Has This Been Tested?
Manual test with latest reva
Screenshots (if appropriate):
Types of changes
Checklist:
Open tasks: