acceptance tests, bugfixes & features #47
Conversation
…lly instantiated powerstrip. Also, factor out useful test setup method into testtools.
…ch turns out breaks half-close hackery). Make failing test actually fail.
…ontent-length. Test for chunked responses.
Turns out, you can't detect 'chunked' encodings from e.g. docker build, because they don't actually use chunked responses. Instead, buffer them (because we have to) only if you have added some post-hooks (and therefore need them to be buffered).
| def dataReceived(self, data): | ||
| # print "DATA!", repr(data) | ||
| return proxy.ProxyClient.dataReceived(self, data) | ||
|
|
There was a problem hiding this comment.
perhaps we want a comment here to indicate what is happening - in my understanding we are overloading this method such that data is sent directly back to the docker client?
Actually - delete these 3 lines because they don't do anything (it turns out) - they just pass through the method call to the superclass
| "\r\n") | ||
| self._streaming = True | ||
| self._fireListener(Failure(NoPostHooks())) | ||
| # print key, "=>", value |
There was a problem hiding this comment.
Remove debugging, or add a debugging mode.
|
After this branch can we please start implementing acceptance tests, bugfixes and features in separate branches. |
| del allRequestHeaders["transfer-encoding"] | ||
| # XXX Streaming the contents of the request body into memory could | ||
| # cause OOM issues for large build contexts POSTed through | ||
| # powerstrip. |
There was a problem hiding this comment.
Raise an isssue for known issues and link to the issue.
|
I haven't reviewed this for correctness. I'm suspicious about the lack of tests for the new chunked encoding stuff. It does have a lot of mess which I would prefer not to be merged. There's also some undocumented technical debt which I've asked that you raise issues for. Please merge after addressing all my comments, thanks. |
…ker pull' runs - commit so I can test it in my VM
|
I've addressed all your concerns (or raised issues in one or two cases). Thanks for the thorough reviews @robhaswell @binocarlos. Merging, thanks! |
acceptance tests, bugfixes & features
docker run,-t,-i, and-tiall work.This is a clear set of improvements which is IMO worthy of a v0.0.2 release.