Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

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

Open
erikkaplun opened this issue Nov 19, 2012 · 2 comments
Open

Comments

@erikkaplun
Copy link
Contributor

(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 pushed a commit to erikkaplun/twistar that referenced this issue Nov 19, 2012
…ot being caught when constructing a new model instance; refs bmuller#30; fixes bmuller#34
erikkaplun pushed a commit to erikkaplun/twistar that referenced this issue Nov 19, 2012
…ot being caught when constructing a new model instance; refs bmuller#30; fixes bmuller#34
@erikkaplun
Copy link
Contributor Author

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

erikkaplun pushed a commit to erikkaplun/twistar that referenced this issue Nov 19, 2012
…turns a Deferredl; refs bmuller#34

This does not ensure backwards compabitility--previously since
afterInit was not called during instantiation, instantiation was never
deferred--but sure does improve it by providing it in cases where
afterInit is not deferred.
@erikkaplun
Copy link
Contributor Author

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.

erikkaplun pushed a commit to erikkaplun/twistar that referenced this issue Nov 19, 2012
erikkaplun pushed a commit to erikkaplun/twistar that referenced this issue Nov 19, 2012
erikkaplun pushed a commit to erikkaplun/twistar that referenced this issue Nov 30, 2012
…ot being caught when constructing a new model instance; refs bmuller#30; fixes bmuller#34
erikkaplun pushed a commit to erikkaplun/twistar that referenced this issue Nov 30, 2012
…turns a Deferredl; refs bmuller#34

This does not ensure backwards compabitility--previously since
afterInit was not called during instantiation, instantiation was never
deferred--but sure does improve it by providing it in cases where
afterInit is not deferred.
erikkaplun pushed a commit to erikkaplun/twistar that referenced this issue Nov 30, 2012
erikkaplun pushed a commit to erikkaplun/twistar that referenced this issue Nov 30, 2012
erikkaplun pushed a commit to erikkaplun/twistar that referenced this issue Nov 30, 2012
…ot being caught when constructing a new model instance; refs bmuller#30; fixes bmuller#34
erikkaplun pushed a commit to erikkaplun/twistar that referenced this issue Nov 30, 2012
…turns a Deferredl; refs bmuller#34

This does not ensure backwards compabitility--previously since
afterInit was not called during instantiation, instantiation was never
deferred--but sure does improve it by providing it in cases where
afterInit is not deferred.
erikkaplun pushed a commit to erikkaplun/twistar that referenced this issue Nov 30, 2012
erikkaplun pushed a commit to erikkaplun/twistar that referenced this issue Nov 30, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant