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.

Sign up for GitHub

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

Jump to bottom

Improved TUS support #88

Closed
3 of 5 tasks
mifi opened this issue Jan 15, 2021 · 4 comments · Fixed by #108
Closed
3 of 5 tasks

Improved TUS support #88

mifi opened this issue Jan 15, 2021 · 4 comments · Fixed by #108
Assignees
@mifi
Labels
enhancement sdks Integrations for Transloadit's API

Comments

@mifi
Copy link
Collaborator

mifi commented Jan 15, 2021 •
edited
Loading

@mifi mifi mentioned this issue Jan 16, 2021
Merged
53 tasks
@Acconut
Copy link
Member

Acconut commented Jan 17, 2021

Implement TUS resume support for Node, or is it not needed? (resume isn't supported for node.js in the current version)

Resuming is supported but just not enabled by default. See https://github.com/tus/tus-js-client/blob/32b3544b7f04601d1d51aaa1eed12921f0a685ec/docs/api.md#urlstorage :)

@mifi
Copy link
Collaborator Author

mifi commented Jan 17, 2021

Sorry, I meant that in the version of tus-js-client that the current node-sdk version is using, resume was not supported. But I upgraded to latest tus-js-client verdion in my PR, and indeed now it’s just a matter of implementing the storage support :) I’m just not sure if resuming is something people would normally need in the node transloadit client.

@Acconut
Copy link
Member

Acconut commented Jan 17, 2021

We normally do not what to implement resuming in the way a UrlStorage can provide since this is not really useful for users of this SDK. What we want is resumability in the sense of overcoming connection issues. But that is already handled by tusd by default in the current versions :) So I think there is nothing to do on your side.

@mifi
Copy link
Collaborator Author

mifi commented Jan 17, 2021

Oh, that's great to hear, thanks!

@lekevbot lekevbot added the sdks Integrations for Transloadit's API label Feb 4, 2021
@kvz kvz assigned mifi Feb 8, 2021
mifi added a commit that referenced this issue Apr 26, 2021
@mifi
Use tus for all uploads
99c01ff 
Now that tusd supports uploading unknown length streams
Fixes #88

- Pull out sendTusRequest into sepearte module so it can be mocked
- Add argument chunkSize (defaults to 5MB only for unknown length streams, Infinity for file streams, same as tus-js-client)
- Emit deprecation warning when isResumable is set to false
@mifi mifi mentioned this issue Apr 26, 2021
Merged
2 tasks
@kvz kvz closed this as completed in #108 May 11, 2021
kvz pushed a commit that referenced this issue May 11, 2021
@mifi
Use tus for all uploads (#108)
e7fe5d4 
* Use tus for all uploads

Now that tusd supports uploading unknown length streams
Fixes #88

- Pull out sendTusRequest into sepearte module so it can be mocked
- Add argument chunkSize (defaults to 5MB only for unknown length streams, Infinity for file streams, same as tus-js-client)
- Emit deprecation warning when isResumable is set to false

* upgrade tus-js-client to v2.3.0

* limit tus upload concurrency (default 10)

* increase tus default chunk size to 50MB
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mifi mifi

Labels
enhancement sdks Integrations for Transloadit's API
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use tus for all uploads transloadit/node-sdk
3 participants
@mifi @Acconut @lekevbot