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

Add support for Range HTTP Header and 206 partial content response. #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jwmay2012
Copy link

What and Why?

To support Range HTTP header and 206 partial content responses.
This is most useful for streaming large files like videos without blocking page loads.

Previous behavior

Before this PR, a frontend would make multiple Range requests expecting partials but resulting in the entire file being downloaded multiple times, all while blocking page load completion.

New behavior

After this PR, the large file gets loaded progressively, as needed by the page, and the Range requests gets a 206 response until the content is completed.

Implementation details

I yanked and adapted the code from http.ServeContent(). It would be preferable to use http.ServeContent() directly, but ServeContent expects a ReadSeeker.

Might be worth it to create an adapter. ServeContent uses ReadSeeker to determine file size and to verify if ranges become out of bounds. However, there's no default code that communicates how many bytes are intended to be read for the current partial content, meaning we'd always have to call objHandler.NewRangeReader(context.Background(), ra.start, attrs.Size) which I suspect pulls the full object from the bucket.

The ServeContent function I pulled from has it's own error response handling which now usurps your handleError(). I left it in, but we can modify it to be consistent.

Thanks for the cool project. We're finding it super useful. :)

@ravwojdyla
Copy link

@jwmay2012 thanks so much for this PR, I have run into the same issue. It would be amazing to get this merged, but in the meantime, if I may ask, have you been using this implementation? Any recommendations or input if others were to use it?

@jwmay2012
Copy link
Author

@ravwojdyla we have been using it in non-prod with the intent to use in prod. Haven't run into any issues or complications. No special deployment considerations as well. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants