Code review comment for lp:~jameinel/bzr/2.3-bzr-connect-ssh

Revision history for this message
Vincent Ladeuil (vila) wrote :

>
> No. The 'start_server' waits until the key negotiation has occurred.

start_server() internally calls Thread.start(), are you sure there is only one
other thread that can set self.completion_event ?

> After that point it does the actual communication on the channel.

>
> The 'ssh_server.join()' waits all the way until the remote end
> disconnects and everything has completed.

That's what I said :)

> The reason this is 'okay' is because the .handle() method is *completely
> empty*, and is just called immediately after .setup().

That also matches what I said, it shouldn't be empty.

It will be clearer to separate between setup, handle and finish, and put the join
into finish.

> The BaseResponseHandler __init__ code is:
>
> self.setup()
> self.handle()
> self.close() # or finish or whatever

Yes, finish() not whatever :)

> That is true, but still what I said above is correct.

Confusingly correct, that's my point.

« Back to merge proposal