[MERGE] Default port for bzr:// (bug 86897)

Andrew Bennetts andrew at canonical.com
Tue Feb 27 03:59:18 GMT 2007


Martin Pool wrote:
> Andrew Bennetts wrote:
> > The attached bundle adds a default port for bzr:// (see
> > https://launchpad.net/bugs/86897).
> 
> Thanks, it'd be good to get this in for 0.15.
> 
> It should have a news entry.

Done.

> > I'm not sure how to test it though.  Ideas welcome.
> 
> We could monkeypatch socket to observe how it is called, but the
> complexity of testing it doesn't seem to me to be justified compared to
> the simplicity of the patch.  So I think it would be ok to just put it
> in and check it manually.

Yeah.  I have of course tested manually, and it works fine.

[...]
> > +++ bzrlib/builtins.py
> > @@ -3128,7 +3128,8 @@
> >          Option('port',
> >                 help='listen for connections on nominated port of the form '
> >                      '[hostname:]portnumber. Passing 0 as the port number will '
> > -                    'result in a dynamically allocated port.',
> > +                    'result in a dynamically allocated port. Default port is '
> > +                    '4155.',
> >                 type=str),
> >          Option('directory',
> >                 help='serve contents of directory',
> 
> It looks like if you don't give a hostname it defaults to 127.0.0.1.
> This is good for security but might be confusing.  (Separate issue I
> guess but should be fixed.)

[...]
> > +            server = smart.SmartTCPServer(t, host=host, port=port)
> >              print 'listening on port: ', server.port
> >              sys.stdout.flush()
> 
> Also it'd be good to show the ip address if any.

Oops, didn't notice these comments until just now.  I'll do that shortly.

John Arbash Meinel wrote:
> John Arbash Meinel has voted +1 (conditional).
> Status is now: Conditionally approved
> Comment:
> I think it would be clearer if 4155 was a constant rather than just a 
> number in the source. So you would do:
> 
> BZR_DEFAULT_SERVE_PORT = 4155
> 
> ...
> 
> if port is None:
>   port = BZR_DEFAULT_SERVE_PORT
>   host = '127.0.0.1'
> elif:
> ...

Done.  (For some reason, I initially couldn't think of a good spot for the
constant, but I since realised that bzrlib/transport/smart.py is the obvious
place.)

-Andrew.




More information about the bazaar mailing list