Opened 8 years ago

Closed 4 years ago

#4223 enhancement closed fixed (fixed)

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

Reported by: David Reid Owned by:
Priority: high Milestone:
Component: web Keywords:
Cc: Glyph, David Reid, ericflo Branch:
Author:

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 8 years ago by Glyph

Owner: changed from jknight to Jean-Paul Calderone
Priority: normalhigh

bump

comment:2 Changed 8 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 8 years ago by Glyph

Cc: Glyph David Reid added

comment:4 Changed 8 years ago by Jean-Paul Calderone

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 8 years ago by David Reid

Cc: ericflo added

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

comment:6 Changed 8 years ago by Jean-Paul Calderone

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 6 years ago by <automation>

Owner: Jean-Paul Calderone deleted

comment:8 Changed 4 years ago by Jean-Paul Calderone

Resolution: fixed
Status: newclosed

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.