TCP client howto missing reactor.run() in one code sample

[itamar]

In the "Simple, single-use clients" section there's a full example, but it's missing a reactor.run().

[exarkun]

 http://twistedmatrix.com/projects/core/documentation/howto/rdbms.html#auto5 is missing it too. I bet lots of other docs are missing it, too.

[thijs]

(In [27112]) Branching to 'examples-reactor-run-3834'

[thijs]

Replying to exarkun:

 http://twistedmatrix.com/projects/core/documentation/howto/rdbms.html#auto5 is missing it too. I bet lots of other docs are missing it, too.

Seems this is missing more than just as a reactor.run() call but also the imports, which is true for all of the examples on that page. If you can fix these imports, please do. Putting up the changes for the original ticket description for review.

[mwh]

I'm not sure adding reactor.run() to the example in clients.xhtml without adding a reactor.stop() as well. It's also a slightly odd example. Do you think changing it to this:

def gotProtocol(p):
    p.sendMessage("Hello")
    defer.maybeDeferred(p.transport.loseConnection).addCallback(reactor.stop)

would make more sense?

All the other changes look good.

[mwh]

Oops, I forgot something: thanks for working on this!

[thijs]

Replying to mwh:

I'm not sure adding reactor.run() to the example in clients.xhtml without adding a reactor.stop() as well. It's also a slightly odd example. Do you think changing it to this: def gotProtocol(p): p.sendMessage("Hello") defer.maybeDeferred(p.transport.loseConnection).addCallback(reactor.stop) would make more sense?

Well, if I do that and run the example it throws this error:

Unhandled error in Deferred:
Traceback (most recent call last):
Failure: twisted.internet.error.ConnectionRefusedError: Connection was refused by other side: 61: Connection refused.

Any idea?

[mwh]

Well, the existing code does that if nothing is listening.

I'm having a hard time getting this correct, so I don't know if I'm a good person to review this branch :) I've got as far as this:

from twisted.internet import reactor
from twisted.internet.protocol import Protocol, ClientCreator
from twisted.python import log

class Greeter(Protocol):
    def sendMessage(self, msg):
        self.transport.write("MESSAGE %s\n" % msg)

def gotProtocol(p):
    p.sendMessage("Hello")
    return p.transport.loseConnection()

c = ClientCreator(reactor, Greeter)
d = c.connectTCP("localhost", 1234)
d.addCallback(gotProtocol).addErrback(log.err).addCallback(
    lambda ignored: reactor.stop())
reactor.run()

But the reactor.stop() gets called before the data is sent.

[mwh]

I should have moved this out of the review state.

[thijs]

assigning to exarkun for help/advice with this ticket.

[exarkun]

Since the code is already doing arbitrary stuff with delayed calls, I think you just want to call reactor.stop after some arbitrary delay that's long enough to let the message be sent most of the time.

That, or make it actually work right, and put the reactor.stop call in the protocol's connection lost (which defeats the purpose of the example, which seems to be about using protocol instances from code that's not part of the protocol class).

So yea, reactor.callLater(3, reactor.stop).

[thijs]

(In [30553]) correct stop call, add news file. refs #3834

[thijs]

Replying to exarkun:

So yea, reactor.callLater(3, reactor.stop).

Merged forward and replaced that call.

[exarkun]

There's a conflict in clients.xhtml now. Can you merge forward and resolve it? Also, the news file doesn't reflect the style described on ReviewProcess#Newsfiles. Can you re-arrange it so it does, and mention the udp and client howtos specifically?

Thanks.

[thijs]

Thanks for the review, i'll look into it.

[thijs]

(In [30594]) Branching to 'examples-reactor-run-3834-2'

[thijs]

Applied the changes in a new branch. Doc builder is  happy.

[jerub]

I've applied this branch to my working copy, read through the examples in the two affected files and while 2 of the examples are runnable, the third simply has an ellipsis and is not runnable - I cut+paste what was required from the other example to splice together something that would work, and it did.

What I'm saying is, I'm glad these examples have been improved, I've tested them, and they work, please merge.

[thijs]

Thanks for the review.

[thijs]

(In [30759]) Added missing reactor.run() calls in the UDP and client howto documents.

Author: thijs Reviewer: mwh, exarkun, jerub Fixes: #3834