Opened 12 years ago
Closed 12 years ago
#18972 closed Bug (fixed)
Refactor bundled WSGI server's chuking logic
| Reported by: | Owned by: | woodm | |
|---|---|---|---|
| Component: | HTTP handling | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description
on core/servers/basehttp.py, lines 97-103:
if length > 33554432:
offset = 0
while offset < length:
chunk_size = min(33554432, length)
self._write(data[offset:offset+chunk_size])
self._flush()
offset += chunk_size
Since only offset is increased, chunksize will always be 33554432, and on the last write might not be the actual amount written. Probably
chunk_size = min(33554432, length-offset)
is better.
Attachments (1)
Change History (15)
comment:1 by , 12 years ago
| Component: | Core (Other) → HTTP handling |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
| Type: | Cleanup/optimization → Bug |
| Version: | 1.4 → master |
comment:2 by , 12 years ago
| Has patch: | set |
|---|
https://github.com/django/django/pull/415
./runtests.py --settings=test_sqlite
returns OK
comment:3 by , 12 years ago
| Keywords: | sprint2013 added |
|---|---|
| Owner: | changed from to |
| Status: | new → assigned |
comment:4 by , 12 years ago
| Keywords: | sprint2013 removed |
|---|---|
| Owner: | removed |
| Status: | assigned → new |
Patch provided above did not include new unit tests.
Rather than send this back marked "needs tests" I created a unit test: https://github.com/django/django/pull/780
I also created an alternative implementation of a bug fix that I suspect may be more efficient: https://github.com/django/django/pull/782 .
Since I contributed code, I cannot bump this to "Ready for Check-In."
I suggest previous patch contributor should resubmit their patch with unit tests, or just pull in the one I made which should be sufficient.
This should still be easy pickings, next owner should be able to simply review and pick which implementation they prefer.
comment:5 by , 12 years ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
comment:7 by , 12 years ago
| Triage Stage: | Ready for checkin → Accepted |
|---|
by , 12 years ago
| Attachment: | 0001-Fixed-18972-Refactored-bundled-wsgi-server-s-chunkin.patch added |
|---|
comment:12 by , 12 years ago
| Summary: | calculation blunder → Refactor bundled WSGI server's chuking logic |
|---|
Added a rebased patch with work from woodm, datagrok and dominik that passes on python 2 and 3.
Note that I refactored the test to assert two chunks were written.
comment:13 by , 12 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
@carljm reviewed the ticket. I will commit the patch soon.
comment:14 by , 12 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Since this was originally written in 2007 - it would be worth looking any improved approaches to this chunking using more recently available Python features.