[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