Modify

Opened 4 years ago

Closed 4 years ago

Last modified 15 months ago

#7262 closed defect (fixed)

db.close() should not be used in trac 0.12

Reported by: ejucovy Owned by: k0s
Priority: normal Component: TracSqlHelperScript
Severity: normal Keywords:
Cc: Trac Release: 0.12

Description (last modified by rjollos)

Don't close the db connection after each SQL command is executed. In Trac 0.12 this causes subsequent db queries to have no connection, causing AttributeErrors.

Patch:

  • tracsqlhelper/__init__.py

     
    2929Exception:%s""" %(sql, params, e)) 
    3030            db.rollback() 
    3131            raise 
    32         try: 
    33             db.close() 
    34         except: 
    35             pass 
     32        #try: 
     33        #    db.close() 
     34        #except: 
     35        #    pass 
    3636        return self.return_values(**_data) 

AFAICT this is a band-aid over a bigger issue. SQLHelperScript does not distinguish between commands that write to the db and commands that only read from it -- all commands will commit and roll-back the database. It also tries to manage its own database connections (get_db_cnx and the now-commented-out db.close()). And in fact it seems like as of Trac 0.12 it should not be trying to manage its own transactions either, or if it does it should be using the with_transaction decorator. But my hunch is that none of this should be managed at the level of the individual SQL command
objects, rather by their callers.

Regardless this small change seems to fix the immediate issue, comment:4:ticket:7182-- though I don't know if it will cause additional problems.

Attachments (1)

connection.diff (724 bytes) - added by ejucovy 4 years ago.
Attaching patch

Download all attachments as: .zip

Change History (8)

comment:1 Changed 4 years ago by ejucovy

I can branch sqlhelperscript for 0.12-compat and commit this patch if you give me access -- I don't seem to have write access on this plugin at trac-hacks. Thanks!

Changed 4 years ago by ejucovy

Attaching patch

comment:2 Changed 4 years ago by anonymous

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

branched for 0.12 compatibility and committed patch on that branch - r8157, r8158, r8159

-ejucovy

comment:3 Changed 15 months ago by rjollos

(In [12978]) Refs #7262: Renamed anyrelease directory to 0.11.

comment:4 Changed 15 months ago by rjollos

  • Description modified (diff)

I'm not sure that [12978] was a good idea, but on the other hand, I don't think it's a good idea to retain a directory named anyrelease, when it no longer works with any release. I've tested that the TracHoursPlugin installs correctly in Trac 0.11 from trachoursplugin/branches/0.11, and I'll do the same for other plugins that utilize the TracSqlHelperScript, after changing their setup script to point to 0.11 rather than anyrelease.

comment:5 Changed 15 months ago by rjollos

(In [12983]) Refs #7262: Bump version of 0.12 branch, so that it differs from the 0.11 branch following [8159].

comment:6 Changed 15 months ago by rjollos

The setup.pys for the following projects point to tracsqlhelperscript/anyrelease, and will need to be modified to point to tracsqlhelperscript/0.11:

comment:7 Changed 15 months ago by rjollos

(In [12984]) Refs #7262: Point setup,py link to TracSqlHelperScript 0.11 directory rather than anyrelease directory.

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from k0s. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.