Opened 3 years ago

Closed 4 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

Version 0, edited 3 years ago by claudep (next)

comment:3 Changed 17 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 11 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 11 months ago by Osmose (previous) (diff)

comment:5 Changed 7 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 7 months ago by Osmose

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

comment:7 Changed 6 months ago by Osmose

  • Patch needs improvement unset

Updated branch with changes in response to feedback.

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

comment:8 Changed 6 months ago by timgraham

  • Patch needs improvement set

There are some problems with the tests on Python 3.

comment:9 Changed 4 months ago by timgraham

  • Patch needs improvement unset

comment:10 Changed 4 months ago by timgraham

  • Triage Stage changed from Accepted to Ready for checkin

comment:11 Changed 4 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