Opened 4 years ago

Closed 2 years ago

#18523 closed New feature (fixed)

Add getvalue to HttpResponse

Reported by: Claude Paroz Owned by: Michael Kelly
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 4 years ago by Aymeric Augustin

Triage Stage: UnreviewedAccepted

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 4 years ago by Claude Paroz

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

Last edited 4 years ago by Claude Paroz (previous) (diff)

comment:3 Changed 3 years ago by Unai Zalakain

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 3 years ago by Michael Kelly

Has patch: set
Owner: changed from nobody to Michael Kelly
Status: newassigned

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 3 years ago by Michael Kelly (previous) (diff)

comment:5 Changed 2 years ago by Tim Graham

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 2 years ago by Michael Kelly

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

comment:7 Changed 2 years ago by Michael Kelly

Patch needs improvement: unset

Updated branch with changes in response to feedback.

Last edited 2 years ago by Michael Kelly (previous) (diff)

comment:8 Changed 2 years ago by Tim Graham

Patch needs improvement: set

There are some problems with the tests on Python 3.

comment:9 Changed 2 years ago by Tim Graham

Patch needs improvement: unset

comment:10 Changed 2 years ago by Tim Graham

Triage Stage: AcceptedReady for checkin

comment:11 Changed 2 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: assignedclosed

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