object_session returns None, but state still has session_id

[Ben Ford]

Hi Folks,

I have the following code:

obj = <some object created by adding to a parent's collection>

ses = object_session(obj)

if not ses:

   ses = Session()

   ses.add(obj)

ses.commit()

This is giving me InvalidRequestError: saying that the object is attached to a session.

When looking into this I have found:

• obj._sa_instance_state.session_id is not in sqlalchemy.orm.session._sessions.
  
• object_session(parent) also returns None
• The session_id given in the InvalidRequestError (i.e <session_id> in this part of the message is already attached to session '<session_id>') == parent._sa_instance_state.session_id

I'm not that familiar with SA's code, but wondered if it would be possible to check for a Session not being valid? Perhaps when the KeyError is caught in the sqlalchemy.orm.session._state_session function (line 1722 in my version) the state session_id could be set to None - or any other action taken that would make the state look like it didn't have a Session.

As for a fix, I guess the best thing to do would be to make_transient(obj) or is there a better way?

Thanks,

Ben

[Ben Ford]

I've now tried a make_transient so my code looks like this:

obj = <some object created by adding to a parent's collection>

ses = object_session(obj)

if not ses:

   make_transient(obj)

   ses = Session()

   ses.add(obj)

ses.commit()

I still get the same exception. 

I guess my other option now is to do:

state = attributes.instance_state(obj)

state.detach()

Would that work do you think? BTW would this be considered a bug, should I raise a bug report on this?

Cheers,

Ben

[Michael Bayer]

I think you're hitting an edge case here that's the result of trying to use objects across sessions where you're just allowing sessions to fall out of scope. I'm not sure the behavior as is is how it should be. The source to _attach() implies that "implicit detachment" was how this worked at some point, or intended to work but there's no tests - I was under the impression that the object is in fact implicitly detached, but that doesn't occur here without a small change. http://www.sqlalchemy.org/trac/ticket/2281 sums up the behavior.

The questions I'd start with, and keep in mind this is towards establishing a use case that would drive the desired behavior, is what is preventing you from keeping a Session in place, or calling close() on it when completed, or using new objects for a new Session instead of recycling those which are hanging around from some previous operation ? That said I'm leaning towards that patch, but probably only in 0.7 as it is a slight behavioral change.

[Ben Ford]

Hi Mike,

It is an edge case for sure. I'm using SA with eventlet so the session is going out of scope in a green thread. Eventlet does some monkeying (literally) around with threadlocals so I get a different session for each one. However in this case the parent instance to which I'm adding obj was fetched in a one greenthread and the child is added in another after a timer has fired off (the child is a record of the timer running). It's definitely fair to say that I'm not using SA in an optimal way, however I'd still be inclined to say that the patch could be worthwhile here.

I should also have mentioned that the original session would have been from a query_property rather than an explicitly created Session().

Is there any particular reason that you added the check where you did in the patch as opposed to the except KeyError clause in _state_session?

Cheers,

Ben

[Michael Bayer]

On Sep 22, 2011, at 5:04 AM, Ben Ford wrote:

Hi Mike,

It is an edge case for sure. I'm using SA with eventlet so the session is going out of scope in a green thread. Eventlet does some monkeying (literally) around with threadlocals so I get a different session for each one. However in this case the parent instance to which I'm adding obj was fetched in a one greenthread and the child is added in another after a timer has fired off (the child is a record of the timer running). It's definitely fair to say that I'm not using SA in an optimal way, however I'd still be inclined to say that the patch could be worthwhile here.

I'm going to do the patch since that's how I'd like it to work, but that is the least of your problems here, if you're linking the scope of your sessions to a library that is repurposing threading to be a system of supplying ad-hoc, transitory context to a longer series of operations.  There's nothing wrong with that approach from a threading point of view, but with this repurposing, it's no longer appropriate to link an operation-scoped object like Session to a thread.

You need to use a contextual callable with scoped_session() that links the lifespan of a Session to the lifespan of the series of objects you're working with - it accepts an argument "scopefunc" - this is a function that should return a "key", usually a string but can be anything hashable, that represents "this is the current context".   For example, if the function was threading.get_ident(), you'd get thread local behavior.

I usually use what I call the "dinner party metaphor" here, the set of objects you're dealing with is your meal, the Session is the plate.   You don't want to keep switching plates for every bite, chucking the previous plate onto the floor with whatever is still stuck to it.   You want to receive your plate, finish your meal, then return the plate.   The thread is usually the dinner guest here but your guest is busy performing on stage, so you need to invite a new guest who knows how to dine properly.

The Session is linked to an ongoing transaction in the database, unless you've turned that off with autocommit=True, and your app should be able to keep track of such a concept, even if eventlet is switching around threads underneath.

If this is going to go the other way, where none of that is possible, then you should be disabling most of Session's automation, turning on autocommit, turning off expire_on_commit, turning off autoflush.  Randomly switching sessions among any subset of objects in a connected graph is generally going to create havoc in any case though.

Is there any particular reason that you added the check where you did in the patch as opposed to the except KeyError clause in _state_session?

The _attach() method of Session, the point at which we determine if an owning Session still exists, doesn't call _state_session().  If you mean why don't we call _state_session() there for the purposes of encapsulation of the session_id attribute, that would imply the whole middle of _attach() just calls _state_session(), which introduces additional function call overhead in a fairly critical section.  _attach() needs to be aware of session_id in any case so it's IMHO fine that it interprets this attribute fully.

SQLAlchemy's code is necessarily inlined in many places due to Python's tremendous function call overhead, some background on that (skip down to the second section, the first half is just wanking) is at http://techspot.zzzeek.org/2010/12/12/a-tale-of-three-profiles/ .