search

bmuller / twistar

Twistar is an object-relational mapper (ORM) for Python that uses the Twisted library to provide asynchronous DB interaction.
http://findingscience.com/twistar
Other
132 stars 38 forks source link

afterInit called twice + errors in it not caught (breaks compat.) #34

Open erikkaplun opened 11 years ago

erikkaplun commented 11 years ago

(I will describe both the issue and the solution here because I already have a patch for this)

After I added a call to afterInit in DBObject.__init__, it (now obviously) started to be called twice: once per createinstances and once per __init__. So I wanted to remove the call to afterInit in createInstances. Seeing the code in createInstances though made me realise that my added line in __init__ swallows errors in afterInit. But afterInit needs to be asynchronous (if not for any practical pruposes—but there are—as per the doc, at least), which means it cannot really be called from __init__ because __init__ is not allowed to return anything (and thus not allowed to return a Deferred). However, __new__ does not suffer from this limitation. So the end solution is to put the call to afterInit into DBObject.__new__.

As a bonus, I also updated the usage of DeferredList in createInstances to include fireOnFirstErrback=True, consumeErrors=True so that any exceptions wouldn't go unnoticed.

erikkaplun commented 11 years ago

(Ignore the first reference—I --force pushed the updated commit so Github is redisplaying it)

erikkaplun commented 11 years ago

The last commit provides backwards compatibility in most cases (i.e. when afterInit does not return a Deferred). The change as a whole does break it but then again—any code that was using afterInit was by definition buggy anyway, unless it had some explicit workarounds, or the previous (inconsistent) behaviour was depended on or happened to not break anything.