Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#2560 closed defect (fixed)

[patch] HttpResponse should support close() for iterators

Reported by: Ivan Sagalaev <Maniac@…> Owned by: Adrian Holovaty
Component: Core (Other) Version:
Severity: normal Keywords:
Cc: Maniac@…, Malcolm Tredinnick Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

WSGI kinda suggests that a server should call 'close()' method of after getting content from a response iterator if it has one. This is very logical since one of the most common examples of an iterator is a file and it really should be closed afterwards. And incidentally 'flup' which is commonly used to run FastCGI server indeed does this.

The problem is that our HttpResponse wraps the actual iterator into a generator (for unicode conversion). And this generator doesn't have 'close()' method and hence the iterator never closed.

I've rewrote this to handle 'close()', patch follows.

Attachments (1)

2560.1.patch (3.8 KB) - added by Ivan Sagalaev <Maniac@…> 10 years ago.
Patch

Download all attachments as: .zip

Change History (7)

Changed 10 years ago by Ivan Sagalaev <Maniac@…>

Attachment: 2560.1.patch added

Patch

comment:1 Changed 10 years ago by Ivan Sagalaev <Maniac@…>

Some comments on the patch:

  • It's somewhat backwards incompatible: before a 'response.iterator' was used to iterator over and now response is an iterator itself. This can be easily fixed by making 'iterator' to return 'self' but I decided to throw it away because it was hardly ever used explicitely and the new approach seems cleaner.
  • The thing that previously was named '_iterator' is now renamed to '_container' which is more pure since an actual iterator is the thing that you get from it be calling 'iter'.

comment:2 Changed 10 years ago by Adrian Holovaty

Summary: HttpResponse should support close() for iterators[patch] HttpResponse should support close() for iterators

comment:3 Changed 10 years ago by Malcolm Tredinnick

Resolution: fixed
Status: newclosed

This was fixed in r3791, but the auto-closer didn't kick in.

comment:4 Changed 10 years ago by Ivan Sagalaev <Maniac@…>

Cc: Malcolm Tredinnick added

Sorry, Malcolm, my English is not deep enough to get the second part of what you've said :-). What's an "auto-closer" and what you mean by "didn't kick in"?

comment:5 Changed 10 years ago by Malcolm Tredinnick

Sorry about that. The "auto-closer" is just my name for whatever it is that automatically reads the svn commit messages and closes the relevant ticket. Sometimes it does not run correctly, even when the commit message is in the right format.

It is completely harmless, don't worry. End result: the bug should be fixed.

comment:6 Changed 10 years ago by Ivan Sagalaev <Maniac@…>

Got it, thanks :-)

Note: See TracTickets for help on using tickets.
Back to Top