-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Static range requests #1382
Static range requests #1382
Conversation
Generally follows https://tools.ietf.org/html/rfc7233 however does not currently support multiple ranges in one request.
Add PR number to change
Revert changelog update to allow tests to run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think aiohttp should provide Range
support on general level
|
||
# Handle 206 range response if requested | ||
if 'range' in request.headers: | ||
from .web_exceptions import HTTPPartialContent, HTTPRequestRangeNotSatisfiable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move it to module level import.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I normally would by standard, but was following what had been done a few lines earlier in the same function. I'll fix both.
end = file_size - 1 | ||
|
||
# Handle 206 range response if requested | ||
if 'range' in request.headers: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think it make sense to move range related functionality to helper module or even to separate module. and create Range
class, it could be useful for external libraries and applications
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had thought that but wasn't sure where would make sense to put it.
It could be in a parse_range_header(request) function in helpers, it's not necessarily making things clearer but would certainly would aid in reuse.
Global range support would be nice sure, but would probably be a significantly larger change. That being said most use cases I know of for range is to get part of static resources for pagination or seeking in video files etc. It's less useful on dynamic content where it's harder to know a range to request of something if the content could change in between requests. |
Remove duplicate code and fix test
@andrewleech your PR is very long waited addition. Couple brief notes:
|
|
…est(), returns (start,end) in format ready to use in a slice of a string/bytearray for convenient reuse Handling specific to file offset and length calculations brought back into send() of FileSender. Similar handling could be implemented in other request handlers depending on type of data being requested.
Please run |
Current coverage is 98.88% (diff: 100%)@@ master #1382 diff @@
==========================================
Files 29 30 +1
Lines 6765 6885 +120
Methods 0 0
Messages 0 0
Branches 1124 1142 +18
==========================================
+ Hits 6683 6808 +125
+ Misses 38 33 -5
Partials 44 44
|
The code looks perfect. |
Ok, I'll update docs. |
Done by c693473 |
Oh, thanks! That's great about the slice object; I didn't know they existed and my initial search to find one didn't come up with anything. I'll keep that in mind for future! |
What do these changes do?
Add support for html range header in static file handling.
Generally follows https://tools.ietf.org/html/rfc7233 however does not support multiple ranges in one request.
Usage is by adding the range header similar to {'Range': 'bytes=0-99'} to a request to receive a partial chunk of the file instead of the entire thing.
Are there changes in behavior for the user?
If the range heading is not explicitly added to a request there should be no change in usage.
Checklist
CONTRIBUTORS.txt
CHANGES.rst
#issue_number
format at the end of changelog message. Use Pull Request number if there are no issues for PR or PR covers the issue only partially.