[MERGE][0.91][Bug 133965] PathNotChild, port mismatch with "bzr info" for bzr:// smartserver

Andrew Bennetts andrew at canonical.com
Thu Sep 13 03:29:52 BST 2007


Vincent Ladeuil wrote:
[...]
>     Andrew> -def register_transport_proto(prefix, help=None, info=None):
>     Andrew> -    transport_list_registry.register_transport(prefix, help, info)
>     Andrew> +def register_transport_proto(prefix, help=None, info=None, default_port=None):
>     Andrew> +    transport_list_registry.register_transport(prefix, help, default_port)
>  
>  
> Using transport_list_registry is not clean:
> - what if ConnectedTransport need to be used without it ?

I'm not sure what your concern here is.  ConnectedTransport is defined in the
same module that initialises transport_list_registry, so transport_list_registry
is always available to ConnectedTransport.  So I don't see a problem here.

> - at least use _get_protocol_handlers which is the API to access it.

I don't understand why this is any better than accessing transport_list_registry
directly, and there's no docstring or other comments to explain what the intent
of this indirection is.

It seems unpythonic to me to have getter and setter functions to get and set an
attribute of a module.  Particularly when those functions are private to that
module (as indicated by the leading underscore), so anything that calls them
could just as easily use the attribute directly.

[...]
> And here.
> 
> I realized you didn't write some of these uses, but there are some
> inconsistencies here.

The inconsistency as far as I can tell is that _get_protocol_handlers exists at
all ;)

I'd love to know what _get_protocol_handlers is meant to achieve.
_{get,set,clear}_protocol_handlers seems to be unused, except for their tests.

So I'd rather not make the change you suggest; it makes things more confusing
for no benefit that I can see.  In fact, I think we should remove the
_*_protocol_handler functions entirely, as they don't seem to be of any use.

Can you explain why you think we should use those functions rather than directly
accessing transport_list_registry?

-Andrew.




More information about the bazaar mailing list