wiki:CodeStyle

Version 2 (modified by dustin, 10 years ago) (diff)

--

Programming with Twisted Python is not difficult, once you've learned the magic that is deferreds. However, writing good code with Twisted can be difficult! The Twisted Coding Standard is a start (although most of it does not apply to Buildbot), but the following are some suggestions for writing usable, readable code:

Prefer to return Deferreds

If you're writing a method that doesn't currently block, but could conceivably block sometime in the future, return a Deferred and document that it does so. Just about anything might block - even getters and setters!

Sequences of Operations

Especially in Buildbot, we're often faced with executing a sequence of operations, many of which may block.

In all cases where this occurs, there is a danger of pre-emption, so exercise the same caution you would if writing a threaded application.

For simple cases, you can use nested callback functions. For more complex cases, deferredGenerator is appropriate.

Nested Callbacks

First, an admonition: do not create extra class methods that represent the continuations of the first:

    def myMethod(self):
        d = ...
        d.addCallback(self._myMethod_2) # BAD
    def _myMethod_2(self, res):
        # ...

Invariably, this extra method gets separated from its parent as the code evolves, and the result is completely unreadable. Instead, include all of the code for a particular function or method within the same indented block, using nested functions:

def getRevInfo(revname):
    results = {}
    d = defer.succeed(None)
    def rev_parse(_): # note use of '_' to quietly indicate an ignored parameter
        return utils.getProcessOutput(git, [ 'rev-parse', revname ])
    d.addCallback(rev_parse)
    def parse_rev_parse(res):
        results['rev'] = res.strip()
        return utils.getProcessOutput(git, [ 'log', '-1', '--format=%s%n%b', results['rev'] ])
    d.addCallback(parse_rev_parse)
    def parse_log(res):
        results['comments'] = res.strip()
    d.addCallback(parse_log)
    def set_results(_):
        return results
    d.addCallback(set_results)
    return d

it is usually best to make the first operation occur within a callback, as the deferred machinery will then handle any exceptions as a failure in the outer Deferred.

Be careful with local variables. For example, if parse_rev_parse, above, merely assigned rev = res.strip(), then that variable would be local to parse_rev_parse and not available in set_results. Mutable variables (dicts and lists) at the outer function level are appropriate for this purpose.

NOTE do not try to build a loop in this style by chaining multiple Deferreds! Unbounded chaining can result in stack overflows, at least on older versions of Twisted. Use deferredGenerator instead.

deferredGenerators

The deferredGenerator API docs explain quite well how to use this class, including an example. Rewriting the above example using deferredGenerator looks like

@defer.deferredGenerator
def getRevInfo(revname):
    wfd = defer.waitForDeferred(utils.getProcessOutput(git, [ 'rev-parse', revname ]))
    yield wfd
    rev = wfd.getResult().strip()
    wfd = defer.waitForDeferred(utils.getProcessOutput(git, [ 'log', '-1',
                                                 '--format=%s%n%b', results['rev'] ]))
    yield wfd
    comments = wfd.getResult().strip()
    yield dict(rev=rev, comments=comments) # return value
                            # (Python does not allow returning a value in a generator)

The great advantage of deferredGenerator is that it allows you to use all of the usual Pythonic control structures in their natural form. In particular, it is easy to represent a loop, or even nested loops, in this style without losing any readability. The downside, of course, is the rather verbose style and the requirement that getResult be called even when no result is needed - this is easy to forget!

inlineCallbacks are not available in Buildbot, because Buildbot is still compatible with Python-2.4.

Joining Sequences

It's often the case that you'll want to perform multiple operations in parallel, and re-join the results at the end. For this purpose, you'll want to use a {{{DeferredList}}}.

def getRevInfo(revname):
    results = {}
    finished = dict(rev_parse=False, log=False)

    rev_parse_d = utils.getProcessOutput(git, [ 'rev-parse', revname ])
    def parse_rev_parse(res):
        return res.strip()
    rev_parse_d.addCallback(parse_rev_parse)

    log_d = utils.getProcessOutput(git, [ 'log', '-1', '--format=%s%n%b', results['rev'] ]))
    def parse_log(res):
        return res.strip()
    log_d.addCallback(parse_log)

    d = defer.DeferredList([rev_parse_d, log_d], consumeErrors=1, fireOnFirstErrback=1)
    def handle_results(results):
        return dict(rev=results[0][1], log=results[1][1])
    d.addCallback(handle_results)
    return d