[WIP] Implement a maximum response size option#902
Conversation
Codecov Report
@@ Coverage Diff @@
## master #902 +/- ##
==========================================
+ Coverage 87.67% 88.59% +0.92%
==========================================
Files 41 41
Lines 5726 5807 +81
Branches 791 807 +16
==========================================
+ Hits 5020 5145 +125
+ Misses 525 482 -43
+ Partials 181 180 -1
|
1b31cc6 to
8cf3788
Compare
8cf3788 to
472237d
Compare
c4d033a to
a836917
Compare
a836917 to
9ed7714
Compare
| return value | ||
|
|
||
|
|
||
| def _size_warrants_abort(sizes_and_sources, render_options, log, reply): |
There was a problem hiding this comment.
this function looks over-complicated, can we improve it somehow?
There was a problem hiding this comment.
Maybe I can extract some of the inner logic into separate functions, such as the parsing of max_size.
| MAX_TIMEOUT = 90.0 | ||
|
|
||
| RESPONSE_SIZE_LIMIT = None | ||
| MAX_RESPONSE_SIZE_LIMIT = None |
There was a problem hiding this comment.
This two looks very alike so a bit confusing, please add a comment about what each they stands for?
BTW why do we need two variables for this limit?
There was a problem hiding this comment.
If I recall right:
-
RESPONSE_SIZE_LIMITis the limit to use as default if the Splash request does not specify one. -
MAX_RESPONSE_SIZE_LIMITis the maximum value that a Splash request may specify. If a higher value is specified by a Splash request,MAX_RESPONSE_SIZE_LIMITis used as limit instead.
It is indeed confusing, I should indeed add a comment.
|
|
||
| def _on_reply_download_progress(self, received, total): | ||
| reply = self.sender() | ||
| request = reply.request() |
| * Tracks information about requests/responses and stores it in HAR format, | ||
| including request and response content. | ||
| * Allows to set per-request timeouts. | ||
| * Handles per-request response size limits. |
There was a problem hiding this comment.
I don't find how to set the "per-request response size limit" in this PR.
There was a problem hiding this comment.
By adding max_size to the render options of a request.
If you mean that the documentation should explain this, indeed.
Gallaecio
left a comment
Gallaecio
left a comment
There was a problem hiding this comment.
Thanks for the early feedback!
However, I don’t think I’ll complete it any time soon, my priorities have switched away from Splash for the time being. If anyone finds this pull request and is interested in completing it, feel free to fork it on a new pull request, and leave a comment about it here.
| MAX_TIMEOUT = 90.0 | ||
|
|
||
| RESPONSE_SIZE_LIMIT = None | ||
| MAX_RESPONSE_SIZE_LIMIT = None |
There was a problem hiding this comment.
If I recall right:
-
RESPONSE_SIZE_LIMITis the limit to use as default if the Splash request does not specify one. -
MAX_RESPONSE_SIZE_LIMITis the maximum value that a Splash request may specify. If a higher value is specified by a Splash request,MAX_RESPONSE_SIZE_LIMITis used as limit instead.
It is indeed confusing, I should indeed add a comment.
| return value | ||
|
|
||
|
|
||
| def _size_warrants_abort(sizes_and_sources, render_options, log, reply): |
There was a problem hiding this comment.
Maybe I can extract some of the inner logic into separate functions, such as the parsing of max_size.
| * Tracks information about requests/responses and stores it in HAR format, | ||
| including request and response content. | ||
| * Allows to set per-request timeouts. | ||
| * Handles per-request response size limits. |
There was a problem hiding this comment.
By adding max_size to the render options of a request.
If you mean that the documentation should explain this, indeed.
2 participants
Based on #58.