-
Notifications
You must be signed in to change notification settings - Fork 486
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
Make use of locker mandatory #1119
Comments
Why not simply add a default in case it's missing? That's what we do in tus Node.js and it wouldn't be a breaking change. |
Maybe I have to differentiate a bit more: For users of the tusd CLI we already add lockers by default, so for them nothing would change. For users of the tusd package, we don't set a default storage and also no default locker since they proper implementation (in-memory, file-based, distributed) depends on their use case. We could default to in-memory locks, but might work well in development and then fail in production without users understanding what is going on. I would prefer if we make our users more aware of this decision they have to make, which would be the case if we error out on a missing lock implementation. Of course, all of this must be accompanied by proper documentation. |
I would argue the exact opposite, accidentally not using a lock is what causes failures in production without users understanding. With Go I'm much more reluctant for major versions than in JS, where you have better version tooling. So with that in mind I'd say preventing a major version at all cost and defaulting to memory locker is better.
If you run a complex distributed setup you are already aware concepts such as lockers need to change. With good docs indicating this, I'd say it's much less likely for an accidental memory locker to cause problems than it is for the majority of people with simpler setups to forget to add a lock if they don't upgrade. |
I think Go made it pretty easy to work with major versions. I would not shy away from major version bumps. |
There's a difference between shying away from major versions for the sake of it and considering alternatives which may prevent it without loosing out on quality. Not matter how you put it, major versions are not ideal. And the main argument was this:
|
Currently, a composer can be created without specifying a lock provider. Such setups should only be used for testing purposes but not for production as explained in https://tus.github.io/tusd/advanced-topics/locks/.
We should make the use of a locker mandatory to make this clear. This is also motivated by questions such as in #1110, where users where unsure whether they need a locker. This change would also allow us to remove some code checking for the existence of a lock provider.
However, this must be done in a major release since this would be a breaking change.
The text was updated successfully, but these errors were encountered: