Opened 3 years ago

Closed 10 months ago

#18523 closed New feature (fixed)

Add getvalue to HttpResponse

Reported by: claudep Owned by: Osmose
Component: HTTP handling Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The HttpResponse object can almost be used as another stream object. Unfortunately, it doesn't offer the getvalue() method to be able to read its content, hence requiring conditional code in some circonstances (see #15197). What about adding it (HttpResponse.getvalue() === HttpResponse.content)?

Change History (11)

comment:1 Changed 3 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

Good idea.

Do Python's docs provide a list of the methods that a "stream object" must support? While we're there, we should also check that HttpRequest objects support all the methods that make sense.

comment:2 Changed 3 years ago by claudep

I think http://docs.python.org/library/io.html#io.IOBase lists basic API for streams.

Last edited 3 years ago by claudep (previous) (diff)

comment:3 Changed 23 months ago by unaizalakain

I would add the following attributes and methods:

  • HttpResponseBase.closed: set to False in __init__, set to True in close()
  • HttpResponseBase.writable(): returns False because write(content) raises an exception (should be changed to IOError BTW)
  • HttpResponseBase.writelines(lines): raises IOError
  • HttpResponseBase.tell(): change it to raise IOError
  • HttpResponse.getvalue(): returns self.content
  • HttpResponse.writelines(lines): writes each line with self.write(line)

comment:4 Changed 17 months ago by Osmose

  • Has patch set
  • Owner changed from nobody to Osmose
  • Status changed from new to assigned

I submitted a PR with the changes mentioned in comment:3 here: https://github.com/django/django/pull/2545 . If we think that all of the stuff from io.IOBase should be included, I can update my PR with the rest.

Last edited 17 months ago by Osmose (previous) (diff)

comment:5 Changed 13 months ago by timgraham

  • Patch needs improvement set

There are some comments for improvement on the PR. Please uncheck "Patch needs improvement" when you update it, thanks.

comment:6 Changed 13 months ago by Osmose

Will do, just having trouble finding a good time to work on this lately. :D

comment:7 Changed 12 months ago by Osmose

  • Patch needs improvement unset

Updated branch with changes in response to feedback.

Last edited 12 months ago by Osmose (previous) (diff)

comment:8 Changed 12 months ago by timgraham

  • Patch needs improvement set

There are some problems with the tests on Python 3.

comment:9 Changed 10 months ago by timgraham

  • Patch needs improvement unset

comment:10 Changed 10 months ago by timgraham

  • Triage Stage changed from Accepted to Ready for checkin

comment:11 Changed 10 months ago by Tim Graham <timograham@…>

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

In ebc8e79cf3bdd42a99e91d6e679248d07097d3db:

Fixed #18523 -- Added stream-like API to HttpResponse.

Added getvalue() to HttpResponse to return the content of the response,
along with a few other methods to partially match io.IOBase.

Thanks Claude Paroz for the suggestion and Nick Sanford for review.

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