Changes between Initial Version and Version 6 of Ticket #7504


Ignore:
Timestamp:
Dec 9, 2010, 8:29:27 AM (13 years ago)
Author:
Ryan J Ollos
Comment:

In particular ​comment:5:ticket:5345 describes essentially the same fix, and several users report that the fix has successfully worked for them. The trunk has since been modified to move cursor = db.cursor() below if self._need_migration(db):, so it seems unlikely that the issue is that _need_migration is creating another cursor and forcing the close of the cursor in environment_needs_upgrade (see also [5655] and #4996). However, from what I've seen in this and other tickets, it seems to be important that the cursor is obtained within the try block.

I'm still trying to understand this issue, but as far as I can tell it would be at worst case harmless to move cursor = db.cursor() to within the try block.

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #7504

    • Property Cc Steffen Hoffmann added; anonymous removed
    • Property Severity changed from normal to blocker
    • Property Summary changed from 'Cannot operate on a closed cursor' eception causes db to appear as if upgrade is needed to 'Cannot operate on a closed cursor' exception causes db to appear as if upgrade is needed

  • Ticket #7504 – Description

    initial v6  
     1I'm trying to configure an existing Trac instance to work from under virtualenv (trac 0.12 with SQLite, wsgi and apache). I ran into a  situation where, after running `trac-admin /path/to/env upgrade` and wiki upgrade, trac, when being called by the webserver says that I have to run the upgrade (i.e. shows an error page to this effect). Running upgrade again on the command line says ''no upgrade necessary''. That is, running trac from command-line and having it called from the webserver (via wsgi) gives two different results to the ''need to be upgraded?'' question.
    12
    2 Trying to configure an existing trac instance to work from under virtualenv (trac 0.12, with wsgi under apache), I ran into a  situation where, after running 'trac-admin /path/to/env upgrade' and wiki upgrade, trac, when being called by the webserver says that I have to run the upgrade.  (ie. shows an error page to this effect)  Running upgrade again on the command line says "no upgrade necessary", i.e. running trac from command-line and having it called from the webserver (via wsgi) gives two different results to the "need to be upgraded?" question.
     3I managed to track this down to the fact that !TagModelProvider's `environment_needs_upgrade` returns ''True'' when being called from wsgi, and ''False'' when being called from the console. The problem appears to be that the `cursor.execute('select count(*) from tags' )` line in there throws an exception, but not because the db is old, but because ''Cannot operate on a closed cursor''.
    34
    4 I managed to track this down to the fact that TagModelProvider's environment_needs_upgrade return 'True' when being called from wsgi, and False when being called from the console. The problem appears to be that the cursor.execute('select count(*) from tags' ) line in there throws an exception, but not because the db is old, but because 'Cannot operate on a closed cursor'.
    5 
    6 (I am using sqlite)
    7 
    8 Moving the 'cursor = db.cursor()' line to below the 'if self._need_migration(db)', ie. to just above the try/except appears to solve this issue.
     5Moving the `cursor = db.cursor()` line to below the 'if self._need_migration(db)', ie. to just above the try/except appears to solve this issue.
    96
    107So, suggestions:
    11 1) the 'except:' line in environment_needs_upgrade() shouldn't be so greedy, i.e. it shouldn't swallow all exceptions (if possible to filter out those caused by an old database)
    12 
    13 2) the 'cursor=db.cursor()' line should be moved to just  above where it is first used.
     8 1. the ''except:'' line in environment_needs_upgrade() shouldn't be so greedy, i.e. it shouldn't swallow all exceptions (if possible to filter out those caused by an old database)
     9 1. the 'cursor=db.cursor()' line should be moved to just above where it is first used.
    1410
    1511Note that I assume the situation I ran into would be very difficult to reproduce, but the suggestions above won't have any drawbacks, and just help make the code cleaner.