Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #1267 (closed defect: fixed)

Opened 10 years ago

Last modified 10 years ago

[PATCH] auto commiting with SQLAlchemy does not work

Reported by: kov Owned by: anonymous
Priority: high Milestone: 1.0.2
Component: TurboGears Version: 1.0
Severity: major Keywords:
Cc:

Description (last modified by jorge.vargas) (diff)

Hello,

This bug has been reported in the TurboGears Debian package ( http://bugs.debian.org/408358):

"After executing a controller method to handle a page request, TurboGears is supposed to commit any changes that the controller made to the database. When using SQLAlchemy, it fails to do this. It's possible to work around this by modifying the controller to do the commit manually, but that's not something that the application should be doing. I'm calling this bug "important" because existing TurboGears-based web applications, as-written, will not work correctly on this release of TurboGears if they use SQLAlchemy.

TurboGears' database support (for both SQLObject and SQLAlchemy) is implemented in the file "turbogears/database.py". This file contains a generic "run_with_transaction" function:

    [dispatch.generic(MultiorderGenericFunction)]
    def run_with_transaction(func, *args, **kw):
        pass

which is supposed to call either so_rwt() or sa_rwt() depending on whether SQLObject or SQLAlchemy is in use:

    [run_with_transaction.when("not _use_sa()")]
    def so_rwt(func, *args, **kw):
        ...

    [run_with_transaction.when("_use_sa()")]
    def sa_rwt(func, *args, **kw):
        ...

The check for whether SQLAlchemy is in use is done by looking at a global variable:

    def _use_sa():
        return _engine is not None

The _engine variable is initialized to None near the top of this file, and a function called get_engine() is provided which may assign an object to it after reading some options from a config file.

The problem is that the so_rwt() and sa_rwt() functions get registered with the run_with_transaction() dispatcher at the same time as they're defined: while Python is executing turbogears/database.py in response to an import declaration. At this point the get_engine() function has been defined, but hasn't been run yet, so _engine is always going to be None, and run_with_transaction() always ends up calling so_rwt(), never sa_rwt().

How to reproduce this: use quickstart to create a new SQLAlchemy project, and then run "tg-admin shell" on it. Do the following:

    import turbogears.database
    turbogears.database.run_with_transaction(None)

This will cause a TypeError? exception, and you'll see so_rwt() in the stack trace.

If I modify turbogears/database.py and put "print _use_sa()" just before the definition of the so_rwt() function, I see False printed when I start a shell, even though running turbogears.database._use_sa() from within the shell yields True. That False is what causes so_rwt() to become the function that the dispatcher uses, rather than sa_rwt() like it should." -- Mike Paul

Attachments

_use_sa.diff Download (508 bytes) - added by tbradshaw 10 years ago.
Instead of checking _engine, now checks for the import and dburi
sa-commit.patch Download (967 bytes) - added by paj 10 years ago.
Proposed fix

Change History

comment:1 Changed 10 years ago by jorge.vargas

  • Priority changed from high to highest
  • Description modified (diff)

this seems very imporant.

also fixed some formatting in the original message.

Changed 10 years ago by tbradshaw

Instead of checking _engine, now checks for the import and dburi

comment:2 Changed 10 years ago by tbradshaw

It appears that get_engine uses the presence of 'sqlalchemy.*' in the configuration file to determine if sqlalchemy should be used or not.

I've modified _use_sa() to do a check on it's own for 'sqlalchemy.dburi' in the configuration file and to make sure that sqlalchemy was imported.

comment:3 Changed 10 years ago by tbradshaw

  • Summary changed from auto commiting with SQLAlchemy does not work to [PATCH] auto commiting with SQLAlchemy does not work

Forgot to change the title.

comment:4 Changed 10 years ago by alberto

  • Priority changed from highest to normal

I've tried reproducing the problem with no success. It seems to me the problem must be elsewhere. This is what I've done (with [2824]):

  1. tg-admin quickstart Foolala --sqlalchemy
  2. cd Foolala
  3. tg-admin shell
  4. import turbogears.database
  5. turbogears.database.run_with_transaction(None)

Traceback:

---------------------------------------------------------------------------
exceptions.ValueError                                Traceback (most recent call last)

/Users/alberto/src/python/checkouts/turbogears/Foolala/<ipython console> 

/Users/alberto/src/python/checkouts/turbogears/Foolala/<string> in run_with_transaction(func, *args, **kw)

/Users/alberto/src/python/checkouts/turbogears/turbogears/database.py in sa_rwt(func, *args, **kw)
    350     except Exception, e:
    351         transaction.rollback()
--> 352         retval = dispatch_exception(e,args,kw)
    353     return retval
    354 

/Users/alberto/src/python/checkouts/turbogears/turbogears/database.py in dispatch_exception(exception, args, kw)
    321     from turbogears.errorhandling import dispatch_error
    322     # Keep in mind func is not the real func but _expose
--> 323     real_func, accept, allow_json, controller = args[:4]
    324     args = args[4:]
    325     exc_type, exc_value, exc_trace = sys.exc_info()

ValueError: need more than 0 values to unpack

It seems sa_rwt is effectively called as expected and turbogears.database._engine is <sqlalchemy.engine.base.Engine object at 0x17a2430>

Besides this, I don't think the order of declaration is important because the dispatcher will always call the callable when evaluating the rule.

Anyone else has experienced this problem? I'm tempted to close as "worksforme" because, well it does work-for-me ;) and I'm pretty sure this would have been reported many times if it was really an issue with TG itself. Maybe there's a problem with the debian package?

Alberto

comment:5 follow-up: ↓ 6 Changed 10 years ago by paj

  • Priority changed from normal to high

I am having this problem too, using the 1.0 svn branch with SQLAlchemy.

I have to manually call session.flush() at the end of my controller methods for changes to be written to the database.

However, applying the patch on this ticket does not solve the problem. Is it intended that TurboGears will automatically flush and commit? If that is the intended behaviour, I can look at producing a patch for this.

comment:6 in reply to: ↑ 5 Changed 10 years ago by alberto

Replying to paj:

I am having this problem too, using the 1.0 svn branch with SQLAlchemy.

I have to manually call session.flush() at the end of my controller methods for changes to be written to the database.

This is weird... auto-flushing and auto-comiting does work on my system... (Mac OSX 10.4.9, Python 2.4.3) with this tg-admin info:

(Turbogears)jal:~ alberto$ tg-admin info
TurboGears Complete Version Information

TurboGears requires:

* TurboGears 1.0.2dev-r2802
* cElementTree 1.0.5-20051216
* configobj 4.3.2
* RuleDispatch 0.5a0.dev-r2247
* setuptools 0.6c5
* FormEncode 0.6
* PasteScript 0.9.7
* elementtree 1.2.6
* simplejson 1.3
* CherryPy 2.2.1
* TurboKid 1.0dev-r2847
* TurboCheetah 0.9.5
* TurboJson 1.0
* PyProtocols 1.0a0dev-r2082
* Cheetah 1.0
* PasteDeploy 0.9.6
* Paste 1.3
* kid 0.9.5
* Cheetah 1.0

Identity Providers 

* sqlobject (TurboGears 1.0.2dev-r2802)
* sqlalchemy (TurboGears 1.0.2dev-r2802)

tg-admin Commands 

* info (TurboGears 1.0.2dev-r2802)
* shell (TurboGears 1.0.2dev-r2802)
* quickstart (TurboGears 1.0.2dev-r2802)
* update (TurboGears 1.0.2dev-r2802)
* sql (TurboGears 1.0.2dev-r2802)
* i18n (TurboGears 1.0.2dev-r2802)
* toolbox (TurboGears 1.0.2dev-r2802)

Visit Managers 

* sqlobject (TurboGears 1.0.2dev-r2802)
* sqlalchemy (TurboGears 1.0.2dev-r2802)

Template Engines 

* cheetah (TurboCheetah 0.9.5)
* json (TurboJson 1.0)
* genshi-markup (Genshi 0.4dev-r512)
* genshi-text (Genshi 0.4dev-r512)
* genshi (Genshi 0.4dev-r512)
* toscawidgets (ToscaWidgets 0.1a2dev-r2777)
* kid (TurboKid 1.0dev-r2847)

Widget Packages 


Toolbox Plugins 

* info (TurboGears 1.0.2dev-r2802)
* catwalk (TurboGears 1.0.2dev-r2802)
* shell (TurboGears 1.0.2dev-r2802)
* designer (TurboGears 1.0.2dev-r2802)
* widgets (TurboGears 1.0.2dev-r2802)
* admi18n (TurboGears 1.0.2dev-r2802)

TurboGears Extensions 

* visit (TurboGears 1.0.2dev-r2802)
* identity (TurboGears 1.0.2dev-r2802)
* toscawidgets (ToscaWidgets 0.1a2dev-r2777)

Maybe you could post yours too?

However, applying the patch on this ticket does not solve the problem. Is it intended that TurboGears will automatically flush and commit?

Yep

If that is the intended behaviour, I can look at producing a patch for this.

That would be appreciated!

Alberto

comment:7 Changed 10 years ago by paj

Ok, here's my info. I'll have a bit more of a dig and see if I can figure out what's going on.

{{{C:\Documents and Settings\Paj>tg-admin info TurboGears Complete Version Information

TurboGears requires:

  • TurboGears 1.0.2dev-r2866
  • celementtree 1.0.5-20051216
  • configobj 4.3.2
  • ruledispatch 0.5a0.dev-r2247
  • setuptools 0.6c5
  • formencode 0.6
  • pastescript 1.0
  • elementtree 1.2.6
  • simplejson 1.4
  • cherrypy 2.2.1
  • turbokid 0.9.9
  • turbocheetah 0.9.5
  • turbojson 0.9.9
  • pyprotocols 1.0a0dev-r2082
  • cheetah 2.0rc7
  • pastedeploy 1.0
  • paste 1.1.1
  • kid 0.9.4
  • cheetah 2.0rc7
  • elementtree 1.2.6

Identity Providers

tg-admin Commands

Visit Managers

Template Engines

  • kid (turbokid 0.9.9)
  • cheetah (turbocheetah 0.9.5)
  • json (turbojson 0.9.9)
  • genshi-markup (genshi 0.4dev-r544)
  • genshi-text (genshi 0.4dev-r544)
  • genshi (genshi 0.4dev-r544)

Widget Packages

Toolbox Plugins

TurboGears Extensions

comment:8 Changed 10 years ago by paj

Ooops :-)

C:\Documents and Settings\Paj>tg-admin info
TurboGears Complete Version Information

TurboGears requires:

* TurboGears 1.0.2dev-r2866
* celementtree 1.0.5-20051216
* configobj 4.3.2
* ruledispatch 0.5a0.dev-r2247
* setuptools 0.6c5
* formencode 0.6
* pastescript 1.0
* elementtree 1.2.6
* simplejson 1.4
* cherrypy 2.2.1
* turbokid 0.9.9
* turbocheetah 0.9.5
* turbojson 0.9.9
* pyprotocols 1.0a0dev-r2082
* cheetah 2.0rc7
* pastedeploy 1.0
* paste 1.1.1
* kid 0.9.4
* cheetah 2.0rc7
* elementtree 1.2.6

Identity Providers

* sqlobject (TurboGears 1.0.2dev-r2866)
* sqlalchemy (TurboGears 1.0.2dev-r2866)

tg-admin Commands

* info (TurboGears 1.0.2dev-r2866)
* shell (TurboGears 1.0.2dev-r2866)
* quickstart (TurboGears 1.0.2dev-r2866)
* update (TurboGears 1.0.2dev-r2866)
* sql (TurboGears 1.0.2dev-r2866)
* i18n (TurboGears 1.0.2dev-r2866)
* toolbox (TurboGears 1.0.2dev-r2866)

Visit Managers

* sqlobject (TurboGears 1.0.2dev-r2866)
* sqlalchemy (TurboGears 1.0.2dev-r2866)

Template Engines

* kid (turbokid 0.9.9)
* cheetah (turbocheetah 0.9.5)
* json (turbojson 0.9.9)
* genshi-markup (genshi 0.4dev-r544)
* genshi-text (genshi 0.4dev-r544)
* genshi (genshi 0.4dev-r544)

Widget Packages


Toolbox Plugins

* info (TurboGears 1.0.2dev-r2866)
* catwalk (TurboGears 1.0.2dev-r2866)
* shell (TurboGears 1.0.2dev-r2866)
* designer (TurboGears 1.0.2dev-r2866)
* widgets (TurboGears 1.0.2dev-r2866)
* admi18n (TurboGears 1.0.2dev-r2866)

TurboGears Extensions

* fastdata (tgfastdata 0.9a6)
* visit (TurboGears 1.0.2dev-r2866)
* identity (TurboGears 1.0.2dev-r2866)

comment:9 Changed 10 years ago by paj

Ok, it is definitely using so_rwt, even though I'm using SQLAlchemy. I notice that _use_sa returns False while the app is starting up, and true while it is running. I wonder if RuleDispatch? is evaluating the "not _use_sa()" when the function is defined (as opposed to, every call). Some crude debugging with print statements seems to support this.

I wondered if this was a RuleDispatch? bug, but we're both using 0.5a0.dev-r2247 and only I have the problem. At this point I need to learn a bit more about generic functions and RuleDispatch?. I'll check back later.

Changed 10 years ago by paj

Proposed fix

comment:10 Changed 10 years ago by paj

That is indeed the problem - seems that because the expression doesn't include any arguments, it is optimized away. I have attached a patch that fixes this, in a somewhat wacky way!

The _use_sa.diff attachment did not fix this for me at all.

BTW, I've not had much luck finding RuleDispatch? docs.  http://peak.telecommunity.com/ hints at being the right place, but I can't find an API reference. Any pointers?

comment:11 Changed 10 years ago by alberto

  • Status changed from new to closed
  • Resolution set to fixed

Applied at [2874]. Thanks! :)

Alberto

Note: See TracTickets for help on using tickets.