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 #80 (closed defect: fixed)

Opened 14 years ago

Last modified 12 years ago

[PATCH] Implicit transaction support

Reported by: anonymous Owned by: kevin
Priority: normal Milestone: 0.9
Component: TurboGears Version:
Severity: blocker Keywords: sqlobject transaction
Cc:

Description

I'm spotting lots of calls to hub.begin(), .commit(), .roolback() in my code and it's starting to get distracting.

Zope has implicit transaction support that runs through the 'transaction' package (not sure if this is in Python core -- maybe just Zope). This is super, super nice! Each transaction is setup with a transaction (typically in Zope to the ZODB) that commits if there's no unhandled exception -- and rollsback if there is.

There are cases when a developer wants to intervene, but 99% the automagical behavior is fine.

Attachments

database.py.patch Download (720 bytes) - added by jal@… 14 years ago.

Change History

comment:1 Changed 14 years ago by michele

Ian Bicking posted this on the google group:

Calling .transaction() gives you a transaction connection, that has to later be used to be of any use. It's already begun, only if you rollback or commit must you call .begin().

Here's the routine I generally use for transactions:

def do_in_transaction(func, *args, **kw):
      old_conn = sqlhub.getConnection()
      conn = old_conn.transaction()
      sqlhub.threadConnection = conn
      try:
          try:
              value = func(*args, **kw)
          except:
              conn.rollback()
              raise
          else:
              conn.commit()
              return value
      finally:
          sqlhub.threadConnection = old_conn

This might be a useful addition to the expose decorator. Actually, I could probably attach the function to ConnectionHub? too -- it ought to exist somewhere in SQLObject.

You can find the complete discussion  here

comment:2 Changed 14 years ago by jonathan-lists@…

This is a great, and very important feature. It is important that it function under any execution model (thread-per-request, process-per-request, or whatever) since TurboGears application will often be deployed through WSGI Servers.

Is there anyone committed to making this happen? If not, is there anyone who knows a little bit about what would need to be done, if its possible, etc?

comment:3 Changed 14 years ago by michele

Another quite interesting post regarding this is  here

It seems worthwhile to quote again Ian Bicking on this:

More proper would be:

hub.begin()
try:
    do stuff
except:
    hub.rollback()
    raise
else:
    hub.commit()

Though of course in the "else:" block you could commit or rollback based
on some flag, if you didn't want to automatically commit. 

IMHO this ticket should really target 0.9 (as decorator parameter) as it is a quite popular argument of discussion and a great opportunity to increase TG DRY rate and help beginners to do transaction always in the proper way.

comment:4 Changed 14 years ago by kevin

  • Milestone set to 0.9

comment:5 Changed 14 years ago by seancazzell@…

  • Owner changed from kevin to seancazzell@…

Michele,

This can be added to expose if everyone is for it. It sounds like a great addition to me. I've assigned this ticket to myself, I'll gladly add it in to expose (for the 0.9 release) with the other expose changes I am making if no one objects.

comment:6 Changed 14 years ago by seancazzell@…

comment:7 Changed 14 years ago by kevin

  • Owner changed from seancazzell@… to kevin

comment:8 Changed 14 years ago by kevin

I didn't realize that Sean was actively working on this and picked it up yesterday while working through another issue (undesirable caching side effects).

The way SQLObject caching works (at present, at least), it is far safer to always be working in a Transaction. Consequently, this is turning into default behavior.

To ensure that you're always working in a Transaction, I have updated the AutoConnectHub? to ensure that it returns a Transaction and have provided new mechanisms to commit/rollback/end all transactions (which will allow multiple database connections).

Once in a Transaction, the next part is to implement the behavior discussed here.

comment:9 Changed 14 years ago by kevin

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

database.py now has a new run_with_transaction function which will commit on success, rollback on failure (HTTPRedirect and InternalRedirect? are not considered failures)

expose now wraps the call to the decorated method with run_with_transaction, but only the *first* exposed method called. This way, calling another method will not cause a commit prematurely.

these changes are in [259]

comment:10 Changed 14 years ago by barry.pederson@…

  • Status changed from closed to reopened
  • Resolution fixed deleted

This commit seems to have broken one of my pages, where I had passed a list of SQLObjects to the Kid template. Now I get an exception (trimmed to fit here)..


.... File "kid/pull.py", line 152, in _track

for p in stream:

File "kid/pull.py", line 179, in _coalesce

for ev, item in stream:

File "/data/home/barryp/cess/cess/templates/resources.py", line 69, in _pull File "<string>", line 1, in <lambda> File "sqlobject/main.py", line 971, in _SO_loadValue

selectResults = self._connection._SO_selectOne(self, dbNames)

File "sqlobject/dbconnection.py", line 787, in getattr

self.assertActive()

File "sqlobject/dbconnection.py", line 728, in assertActive

assert not self._obsolete, "This transaction has already gone through ROLLBACK; begin another transaction"

AssertionError?: This transaction has already gone through ROLLBACK; begin another transaction


All my Kid template is doing with the SQLObjects is reading a couple properties (like <span py:content="r.name">Name</span>, where r is an SQLObject)

Would it make sense in controllers.py to have the call to

controllers._process_output(tg_format, output, html)

be part of the transaction?

comment:11 Changed 14 years ago by barry.pedeson@…

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

Revision [278] fixed the "..already gone through ROLLBACK" problem during Kid template processing.

comment:12 Changed 14 years ago by jal@…

  • Status changed from closed to reopened
  • Keywords sqlobject transaction added
  • Resolution fixed deleted
  • Severity changed from normal to blocker

Transactions are giving me problems with Catwalk, though I got the same error in one of my controllers running with wsgi (cherrypy standalone fixed it). I get this error when I try to view one of my sqlobject classes:

2005/12/13 12:52:22  INFO Traceback (most recent call last):
  File "/usr/lib/python2.4/site-packages/CherryPy-2.1.0-py2.4.egg/cherrypy/_cphttptools.py", line 271, in run
    main()
  File "/usr/lib/python2.4/site-packages/CherryPy-2.1.0-py2.4.egg/cherrypy/_cphttptools.py", line 502, in main
    body = page_handler(*args, **cherrypy.request.paramMap)
  File "/root/turbogears/turbogears/controllers.py", line 196, in newfunc
    html, *args, **kw)
  File "/root/turbogears/turbogears/database.py", line 174, in run_with_transaction
    retval = func(*args, **kw)
  File "/root/turbogears/turbogears/controllers.py", line 222, in _execute_func
    output = func(self, *args, **kw)
  File "/root/turbogears/turbogears/catwalk/catwalk.py", line 969, in columns
    columns=self.object_columns(objectName),
  File "/root/turbogears/turbogears/catwalk/catwalk.py", line 475, in object_columns
    cols = self.get_columns_for_object(object)
  File "/root/turbogears/turbogears/catwalk/catwalk.py", line 487, in get_columns_for_object
    cols.append(self.get_column_properties(column_name,column))
  File "/root/turbogears/turbogears/catwalk/catwalk.py", line 449, in get_column_properties
    props = self.get_foreign_key_properties(column,props)
  File "/root/turbogears/turbogears/catwalk/catwalk.py", line 464, in get_foreign_key_properties
    properties['labelColumn'])
  File "/root/turbogears/turbogears/catwalk/catwalk.py", line 324, in foreign_key_alternatives
    alt = list(obj.select())
  File "/usr/lib/python2.4/site-packages/SQLObject-0.7.0-py2.4.egg/sqlobject/sresults.py", line 149, in __iter__
    return iter(list(self.lazyIter()))
  File "/usr/lib/python2.4/site-packages/SQLObject-0.7.0-py2.4.egg/sqlobject/sresults.py", line 157, in lazyIter
    return conn.iterSelect(self)
  File "/usr/lib/python2.4/site-packages/SQLObject-0.7.0-py2.4.egg/sqlobject/dbconnection.py", line 755, in iterSelect
    select, keepConnection=True)))
  File "/usr/lib/python2.4/site-packages/SQLObject-0.7.0-py2.4.egg/sqlobject/dbconnection.py", line 687, in __init__
    self.dbconn._executeRetry(self.rawconn, self.cursor, self.query)
  File "/usr/lib/python2.4/site-packages/SQLObject-0.7.0-py2.4.egg/sqlobject/dbconnection.py", line 295, in _executeRetry
    return cursor.execute(query)
ProgrammingError: ERROR:  current transaction is aborted, commands ignored until end of transaction block

BTW, my db backend is PostgreSQL

I'm new to TG so I still need to investigate a bit... but I'll try to fix and submitt a path (though any help would be appreciated! ;)

comment:13 Changed 14 years ago by jal@…

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

Attached is a patch that fixes the problem I mentioned. Apparently ending a connection wasn't raising AutoConnectHub?'s obsolete flag.

Changed 14 years ago by jal@…

comment:14 Changed 14 years ago by michele

  • Summary changed from Implicit transaction support to [PATCH] Implicit transaction support

comment:15 Changed 14 years ago by anonymous

  • Status changed from closed to reopened
  • Resolution fixed deleted

comment:16 Changed 13 years ago by kevin

I'm not sure about this patch. It's setting _obsolete on the non-Transaction connection that the AutoConnectHub? originally had. I haven't taken a look at CatWalk's code, but I'm assuming that the problem has to do with redundant transaction handling there. (We need to be sure that our code is safe from redundant transaction handling, though, because that will be common when moving from 0.8 to 0.9.)

comment:17 Changed 13 years ago by kevin

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

I think this is the same as the problem reported in #225, which has now been fixed.

Note: See TracTickets for help on using tickets.