Metadata Objects get detached from session with repoze-plugins/sa

[Bernie]

My application appears to be experiencing a nasty issue which appears to be the result of removing the session in _BaseSQLAlchemyPlugin.get_user.

My application makes use of the SA metadata provider and loads sqlalchemy objects into the indentity dictionary for use in the application. As a result of closing the database session, these metadata objects become detached and the application fails as soon as it attempts to resolve any relations from those objects.

Although the session.remove is happening at the beginning of the request, the metedata objects are still getting detached as my application has a few repoze middleware plugins that run on ingress and egress during the request.

Rather than removing the session in the repoze plugin, it would be better if the session was removed in a separate middleware layer on egress after both the pylons application and the repoze middleware have completed. This has the added benefit of using only one session per request rather than potentially opening and closing multiple sessions as the request moves from repoze plugins to the application.

[gnarea]

Hello, Bernie.

Thanks for taking the time to report this.

I think you're right. I considered implementing it in the first place [1], but it started to turn into something common, that I decided to implement that fix which would work in probably 98% of the situations.

I'm going to add the middleware I described in [1] to this package and then I'm going to make this behavior (removing the session) optional. I'll also update the documentation to emphasize this option.

How does that sound? In the meant time, you can try giving this plugin a SA session which is not shared with anything else.

Cheers.

[1] http://www.mail-archive.com/pylons-discuss@googlegroups.com/msg15888.html

[Cito]

Hi Gustavo, Bernie, today I also stumbled over this issue. In one of my TurboGears 2 apps I had added a custom authenticator that added or changed user instances in the local database when it found them in an LDAP directory. After updating repoze.who.plugins.sa, this stopped working because the session is now removed in the get_user() method. I solved this by adding a transaction.commit() after the authenticator but I really think removing the session should be made optional. It also took me quite a while to find the culprit!

Also, when looking at that method, I wondered whether it would be worthwile to improve performance by getting the user with query.get() instead of query.filter(...).one(), because in the former query SQLAlchemy can get the user instance from the session without accessing the database, while in the latter query it always needs to access the database. Since get_user() is usually called 4 in a request, this may speed up things a bit. However, for this to work, the get_user() method would need to store the primary key somewhere in the environment on the first call. Does this make sense?

[gnarea]

Hello Christoph,

Both of your suggestions make sense to me, although I'm not so keen on the second one, to be honest, because I see it as a potential violation to the architecture of repoze.who. Ideally, repoze.who would provide this facility (and I think this situation is improved to some extent in repoze.who 2).

Being pragmatic seems like the way to go here though, so I think we should just go ahead. I suppose the "identity" dict would be a better place to store this value, in the (unlikely) event that you have an embedded WSGI app and both apps use repoze.who-sqlalchemy (in which case both apps would get fresh "identity" dicts).

If you could put a patch together, that'd be much appreciated. I'm over-committed for the rest of the week to other things and it wouldn't be wise for me to take on one more thing! :)

Cheers,

Gustavo.

[Cito]

Will try to work out a patch, but it may take a while as I'm busy with other things, too.