[20 PATCHES] Wire protocol unification

Dirkjan Ochtman dirkjan at ochtman.nl
Wed Jul 23 12:01:01 CDT 2008


On Wed, Jul 23, 2008 at 18:28, Matt Mackall <mpm at selenic.com> wrote:
> __all__ can be "a string that gets".split()

I don't much like that convention, but sure, if you prefer it.

> the req.clength thing is quite weird

We need a protocol-independent way to specify the length of the
content. It's weird in the protocol, because some commands use it and
some don't. I think a new protocol version would be good, but that is
for some other time. My patches only try to find a nice way to unify
the existing code into a somewhat coherent set of functions.

> why are we importing from hgweb?

We need some HTTP status codes for unbundle error responses. I could
redefine them in protocol.py if you think that's a better solution. I
thought about doing so myself, but decided not to in the end on the
basis that some or several of these are already available as constants
in hgweb.common.

> import changegroup as changegroup_, please

I thought you didn't like underscores? ;)

> we've got ui.push for I/O redirection (death to cStringIO)

ui.pushbuffer() only buffers stdout, not stderr. There was a bug about
stderr for the http unbundle. I thought about making pushbuffer buffer
stderr as well as stdout optionally per caller, do you think that's a
good idea?

> five layers of try: in a single function has got to be some sort of
> record

This is mostly due to how Python < 2.5 can't have finally and except
for the same try block. I guess I could unify the finally blocks.
Maybe the inner except isn't needed. But these things are already in
the hgweb.protocol code, so I think they should be taken care of
separately (though I agree wholeheartedly with your sentiment and
would like to fix them myself!), that is, not hold up this
unification.

> I'm not sure how I feel about using __all__ like that. Is there any
> reason not to use something like commands?

That would be fine with me. I remember discussing this in IRC, we
picked __all__ because it's a somewhat related Python convention.

Cheers,

Dirkjan


More information about the Mercurial-devel mailing list