-
Notifications
You must be signed in to change notification settings - Fork 487
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
handler: support HTTP range requests in GET /:upload #1064
Comments
As mentioned in #1048, I prefer having explicit support for fetching ranged content inside the storage implementations, so that we can implement efficiently. My first idea was to add a new interface for storage, such as:
The slices are necessary because the However, after looking at the source code of Go's implementation for ranged GET requests (see https://cs.opensource.google/go/go/+/refs/tags/go1.21.6:src/net/http/fs.go;l=169-355), I am more hesitant with this approach. Go implements additional logic for preventing abuse and ensuring wide compatibility with even odd clients. If we go with my above proposal, we would have to duplicate this logic inside tusd as well, which I would like to avoid. As an alternative, we could go with a more lazy route and basically pass the request and response into the storage to let it handle the ranged request as efficiently as possible:
The Let me know what you think! |
KISS. I am unsure if multiple range requests are really common in a request, and I already had to deal with If needed, revisit when someone asks, and then/or make a 3rd interface. Let the backends deal with the problem, and don't try to be too smart/know better than the storage systems without a good reason. |
Agreed. I am not sure when I will have time to work on this. If anybody is interested in starting this, let me know and I can help :) |
@Acconut im looking at your suggestion and would like to know how |
The data stores could implement an additional interface that turns a regular Upload into a ServableUpload, similar to how we already have TerminaterDataStore; type ServableUpload interface {
ServeContent(ctx context.Context, w ResponseWriter, r *Request) (error)
}
type ServerDataStore interface {
AsServableUpload(upload Upload) ServableUpload
} If a data store does not implement |
Do we then want
as well? then call it conditionally like the others? |
Yes, the composer needs to be adjusted as well, but take the data store interface instead of the additional interface for the upload: func (store *StoreComposer) UseServer(ext ServerDataStore) {
store.UsesServer = ext != nil
store.Server = ext
} (although we would have to think about the use of "server". While it's custom in Go to use -er suffix for interfaces, the use of "server" can easily be misunderstood here) |
…serving, closes tus#1064 - Add ServerDataStore interface and S3ServableUpload implementation - Update handlers to use ServerDataStore when available - Implement range request handling for S3ServableUpload - Add tests for new ServerDataStore functionality - Update Go version to 1.22.1
…ontent serving, closes tus#1064 - Add ServerDataStore interface - Update handlers to use ContentServerDataStore when available - Implement range request handling for s3upload - Add tests for new ContentServerDataStore functionality - Update Go version to 1.22.1
…ontent serving, closes tus#1064 - Add ServerDataStore interface - Update handlers to use ContentServerDataStore when available - Implement range request handling for s3upload - Add tests for new ContentServerDataStore functionality - Update Go version to 1.22.1
…ontent serving, closes tus#1064 - Add ServerDataStore interface - Implement ContentServerDataStore in S3Store with streaming support - Add Range header support for partial content requests - Update StoreComposer to support ContentServer capability - Add tests for new ContentServerDataStore functionality - Update Go version to 1.22.1
…ontent serving, closes tus#1064 - Add ServerDataStore interface - Implement ContentServerDataStore in S3Store with streaming support - Add Range header support for partial content requests - Update StoreComposer to support ContentServer capability - Add tests for new ContentServerDataStore functionality - Update Go version to 1.22.1
Support for ranged GET requests in filestore and s3store has just landed. |
Some discussion around API design in the quoted (since-closed) PR.
Originally posted by @Acconut in #1048 (comment)
The text was updated successfully, but these errors were encountered: