Fixed issues #30, #32, #33 and #34

[erikkaplun]

No description provided.

[bmuller]

Thanks for these! I'll merge them in this weekend.

[erikkaplun]

Actually there is a problem: if afterInit is called in __init__, it is hard to catch any deferred exceptions in it.

Also, the call to afterInit must be removed in createInstances—otherwise it gets called twice. (this is how i discovered the exception catching issue.

A workaround would be to override __new__ and make it return a Deferred, which then returns an instance, so that instead of:

 obj = MyObj()

we'd have

 obj = yield MyObj()  # assuming the use of defer.inlineCallbacks

This might even be a nice complement to the idea of a fully ascynhronous ORM—even object instantiation would be asynchronous.

[bmuller]

Hey eallik - these changes look good but they seem to break 28 tests.

[erikkaplun]

One might expect me to have updated the tests as well, I agree, but nevertheless I explicitly stated that the changes are not backwards compatible, so yeah... Can you perhaps look into it and see if there's something trivial to make most of them pass again? I'd look into the non trivial ones. It's most probably the circumstance that constructing an object now in general returns a deferred (where ever afterInit returns one).

[erikkaplun]

...just nevermind my previuos comment—tests pass now :) (except the failure described in Issue #35 for which I've sent you another pull request so you could merge that independently of the changes here).

[erikkaplun]

Oh and I think this should cause a version bump to 1.1 because of the backwards incompatible change in the behavior of afterInit and model object instantiation (if you decide to merge, that is).

[erikkaplun]

Ping?

[bmuller]

I need some time to take a look at this in detail. Your changes break all backwards compatibility, and that's not something I'm going to accept lightly (and therefore not quickly). Hopefully I'll have some time to look at it this weekend or next.

My intent was to always make object instantiation synchronous, and only produce a deferred when there is necessary interaction w/ the DB. This represents a distinct departure from that intention - and I need to take some time to weigh the benefits / consequences.

[erikkaplun]

Actually there is a conceptual/design problem with afterInit still. Basically the same afterInit for both after loading an existing record from the DB and after creating a completely new unsaved instance does not seem to work. E.g. if I want to apply json.loads to a JSON field. But at the same time, there needs to be a place where you do stuff that you normally do in __init__. But the doc says to use afterInit. But afterInit is not currently called by __init__.

So I'd like to propose a better solution. This solution is not backwards compatible because it requires afterInit to be renamed to afterLoad, but at least instance creation is synchronous.

• afterInit gets renamed to afterLoad and its semantics remain as they are (in your code, not in my patch).
• afterLoad will continue to be asynchronous and thus not called during object instantiation.
• We add a new afterInit which is expected to be synchronous. It gets called during either only fresh object instantiation, or during both instantiation and loading from the DB.

There might be issues with this approach—if you can point out any, I'd be happy.