Opened 5 years ago

Closed 20 months ago

#4223 enhancement closed fixed (fixed)

twisted.web._newclient should not unconditionally send "Connection: close"

Reported by: dreid Owned by:
Priority: high Milestone:
Component: web Keywords:
Cc: glyph, dreid, ericflo Branch:
Author: Launchpad Bug:

Description

Currently it does this here: http://twistedmatrix.com/trac/browser/trunk/twisted/web/_newclient.py#L538

This makes it impossible to build an Agent like object without patching _newclient.

Change History (8)

comment:1 Changed 5 years ago by glyph

  • Owner changed from jknight to exarkun
  • Priority changed from normal to high

bump

comment:2 Changed 5 years ago by glyph

Also, if there's some reason that Connection: close should really be set sometimes, why isn't it in self.headers? It seems like it would be nice for the client API to be able to inspect the Request to at least tell that it's there.

comment:3 Changed 5 years ago by glyph

  • Cc glyph dreid added

comment:4 Changed 5 years ago by exarkun

I'm not sure which self.headers you mean. Probably the answer is something like "Because self.headers is for end-to-end headers, and Connection: close is a hop-by-hop header", but that could be wrong.

I think dreid had a friend who was working on something like this? Maybe he'd like to be the owner of this ticket.

comment:5 Changed 5 years ago by dreid

  • Cc ericflo added

My gut reaction is that this should just be parameterized as a self.shouldClose on the Request.

comment:6 Changed 5 years ago by exarkun

Probably not a Request attribute. I think one of these approaches would be better (in order of preference):

  1. Don't ever send Connection: close. Instead, require whoever's using HTTP11ClientProtocol to call loseConnection directly.
  2. Make it a parameter to HTTP11ClientProtocol.__init__.
  3. Make it a parameter to HTTP11ClientProtocol.request

The third of these is closest to the suggestion to add it as an attribute of Request, but it's slightly better because it doesn't involve widening the request interface with something that's not really related to the request (but rather related to the transport over which the request is issued).

comment:7 Changed 4 years ago by <automation>

  • Owner exarkun deleted

comment:8 Changed 20 months ago by exarkun

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

It seems that doing this separately from the high-level API was skipped. #3420 added HTTPConnectionPool which convinces the lower-level parts of the implementation to use persistent connections.

Note: See TracTickets for help on using tickets.