Code review comment for lp:~mbp/bzr/497274-http-405

> This tries to handle 'http 405 not allowed' from Google code as meaning "not
> allowed to look for .bzr", ie there's no bzr branch there. That may allow
> foreign branches to work against it. However, their servers keep falling over
> with '503 not available' errors, which make testing it difficult.

Then look at lp:~vila/bzr/497274-http-405 for an alternative, minimal (and probably incomplete)
fix with tests.

_post() is used only by smart_http_request() and the later already translate
InvalidHttpResponse into SmartProtocolError which higher layers interpret as NotABranch when needed.

>
> I've only tested this interactively not added tests. I probably should.

See above.

>
> This prints and logs the actual message from the http response page.

I doubt users are really interested in the general case, muttering may be enough though.

>
> It also tries to unify http response handling currently scattered across
> _pycurl.py into one method in the base class.

I'm pretty sure I went mostly in the opposite direction in the past based on problems
encountered while implementing the webdav support.

I didn't try to unify both implementations either... mostly because we will deprecate pycurl
one day and I was happy with the urllib one. The idea there is that there are three categories
of error codes:
- the ones that represent success,
- the ones that represent known failures,
- the others

Upon reception, the callers must handle the success cases, may handle some known
failures specifically before relying on a fallback for the remaining ones.

Basically the http requests are generic enough that the same error code can be interpreted
differently in different contexts. Unifying the error handling too much makes it harder
to raise the correct bzr exceptions.

So far, only 403 was really unambiguously common, even 404 needed to be handled separately
for HEAD (which is the main reason why I prefer to *not* have it in the common code).

All in all, that gives a coherent way to code all the methods with limited duplication
(mostly 404 but everybody knows what 404 means :)

> This may be slightly dangerous
> to change as it's lightly tested and tries to work around server quirks, but
> any breakage should be shallow and it's worth clearing it up I think. I
> haven't yet gone through the urllib implementation.

See above, that's a one-line fix, the pycurl implementation doesn't even need to be fixed.

That may explains why you can't test it against google, I suspect others use the
urllib implementation and needs the fix.

Note that I fixed a bug in send_http_smart_request so you want to backport at least that part.

So I vote needs_fixing and will mark as Work In Progress.