Skip to content
This repository was archived by the owner on Jun 20, 2024. It is now read-only.

[proxy] rewrote chunked response handler #1110

Closed
wants to merge 2 commits into from
Closed

Conversation

paulbellamy
Copy link
Contributor

  1. We cannot send "Connection: close", because the fsouza docker client
    expects the tcp socket to stay open between requests.

  2. Because we cannot force-close the connection, we can't hijack the
    connection (because go's net/http doesn't let use un-hijack it).

  3. Because we need to maintain the individual chunking of messages (for
    docker-py), we can't just copy the response body, as Go will remove and
    re-add the chunking willy-nilly.

Therefore, we have to read each chunk one-by-one, and flush the
ResponseWriter after each one.

Fixes #1103

Seems like quite a large change for a patch release. So, should probably consider merging into master instead.

@paulbellamy paulbellamy force-pushed the 1103-proxy-chunking branch 3 times, most recently from 22de89f to 230fc42 Compare July 10, 2015 11:09
@rade
Copy link
Member

rade commented Jul 10, 2015

reviewing this would be easier if there was a commit that introduces the bits of code from net/http/internal unaltered, and then a separate commit that changes them.

1) We cannot send "Connection: close", because the fsouza docker client
   expects the tcp socket to stay open between requests.

2) Because we cannot force-close the connection, we can't hijack the
   connection (because go's net/http doesn't let use un-hijack it).

3) Because we need to maintain the individual chunking of messages (for
   docker-py), we can't just copy the response body, as Go will remove and
   re-add the chunking willy-nilly.

Therefore, we have to read each chunk one-by-one, and flush the
ResponseWriter after each one.
@paulbellamy paulbellamy force-pushed the 1103-proxy-chunking branch from 230fc42 to f703fd1 Compare July 10, 2015 12:54
@paulbellamy
Copy link
Contributor Author

Going to change this to merge into master instead of 1.0, as it is a large change.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants