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

assets: add range request support for FileHandler #628

Merged
merged 3 commits into from
Oct 13, 2023

Conversation

jpelkonen
Copy link
Contributor

This adds range request support for FileHandler. This is a critical feature for handling large files, or any application that will require range request support, e.g. video or audio streaming.

This implementation currentyl only supports ranges that result in a single response, i.e. multiple ranges are not supported.

This implementation will return an error if a range is open from both ends, or if the range beginning is larger than the end.

This adds range request support for FileHandler. This is a critical
feature for handling large files, or any application that will require
range request support, e.g. video or audio streaming.

This implementation currentyl only supports ranges that result in a
single response, i.e. multiple ranges are not supported.

This implementation will return an error if a range is open from both
ends, or if the range beginning is larger than the end.

Signed-off-by: Janne Pelkonen <jpelkonen@ideanovatech.com>
Copy link
Member

@msrd0 msrd0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR! I've added a few comments, the rest looks really good

gotham/src/handler/assets/mod.rs Outdated Show resolved Hide resolved
gotham/src/handler/assets/mod.rs Outdated Show resolved Hide resolved
gotham/src/handler/assets/mod.rs Outdated Show resolved Hide resolved
gotham/src/handler/assets/mod.rs Outdated Show resolved Hide resolved
…am-rs#628

Specifically:

- Use pattern match instead of unwrap
- Simplify resolve_range by reducing nesting
- Use doc comments for functions
- Use method format for seek

Signed-off-by: Janne Pelkonen <jpelkonen@ideanovatech.com>
@jpelkonen
Copy link
Contributor Author

@msrd0 I appreciate your timely feedback and have addressed the issues raised in my latest commit. Thanks!

@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Merging #628 (9ec7934) into main (702a03a) will increase coverage by 0.40%.
The diff coverage is 97.67%.

@@            Coverage Diff             @@
##             main     #628      +/-   ##
==========================================
+ Coverage   78.33%   78.73%   +0.40%     
==========================================
  Files          72       72              
  Lines        2049     2088      +39     
==========================================
+ Hits         1605     1644      +39     
  Misses        444      444              
Files Coverage Δ
gotham/src/handler/assets/mod.rs 93.01% <97.67%> (+3.35%) ⬆️

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

…otham-rs#628

Use method format for seek

Signed-off-by: Janne Pelkonen <jpelkonen@ideanovatech.com>
@msrd0 msrd0 enabled auto-merge (squash) October 13, 2023 18:26
@msrd0 msrd0 merged commit 7ea4025 into gotham-rs:main Oct 13, 2023
11 checks passed
@jpelkonen jpelkonen deleted the add-range-request-file-serving branch October 13, 2023 19:00
@jpelkonen jpelkonen restored the add-range-request-file-serving branch October 24, 2023 21:50
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.

None yet

2 participants