Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#2560 closed defect (fixed)

[patch] HttpResponse should support close() for iterators

Reported by: Ivan Sagalaev <Maniac@…> Owned by: adrian
Component: Core (Other) Version:
Severity: normal Keywords:
Cc: Maniac@…, mtredinnick 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@…> 9 years ago.
Patch

Download all attachments as: .zip

Change History (7)

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

Patch

comment:1 Changed 9 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 9 years ago by adrian

  • Summary changed from HttpResponse should support close() for iterators to [patch] HttpResponse should support close() for iterators

comment:3 Changed 8 years ago by mtredinnick

  • Resolution set to fixed
  • Status changed from new to closed

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

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

  • Cc mtredinnick 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 8 years ago by mtredinnick

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 8 years ago by Ivan Sagalaev <Maniac@…>

Got it, thanks :-)

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