Drop SQLite2 support

[anatoly techtonik <techtonik@…>]

When trying to run 0.11.x Trac on an old sqlite2 DB the following error is displayed.

Traceback (most recent call last):
  File "m:\p\trac\trac-0.11dev\trac\web\api.py", line 377, in send_error
    'text/html')
  File "m:\p\trac\trac-0.11dev\trac\web\chrome.py", line 727, in render_template
    req.chrome[type_].append(
  File "m:\p\trac\trac-0.11dev\trac\web\api.py", line 195, in __getattr__
    value = self.callbacks[name](self)
  File "m:\p\trac\trac-0.11dev\trac\web\chrome.py", line 488, in prepare_request
    for category, name, text in contributor.get_navigation_items(req):
  File "m:\p\trac\trac-0.11dev\trac\ticket\web_ui.py", line 163, in get_navigation_items
    if 'TICKET_CREATE' in req.perm:
  File "m:\p\trac\trac-0.11dev\trac\perm.py", line 527, in has_permission
    return self._has_permission(action, resource)
  File "m:\p\trac\trac-0.11dev\trac\perm.py", line 541, in _has_permission
    check_permission(action, perm.username, resource, perm)
  File "m:\p\trac\trac-0.11dev\trac\perm.py", line 428, in check_permission
    perm)
  File "m:\p\trac\trac-0.11dev\trac\perm.py", line 285, in check_permission
    get_user_permissions(username)
  File "m:\p\trac\trac-0.11dev\trac\perm.py", line 360, in get_user_permissions
    for perm in self.store.get_user_permissions(username):
  File "m:\p\trac\trac-0.11dev\trac\perm.py", line 178, in get_user_permissions
    cursor.execute("SELECT username,action FROM permission")
  File "m:\p\trac\trac-0.11dev\trac\db\util.py", line 60, in execute
    return self.cursor.execute(sql)
  File "m:\p\trac\trac-0.11dev\trac\db\sqlite_backend.py", line 58, in execute
    args or [])
  File "m:\p\trac\trac-0.11dev\trac\db\sqlite_backend.py", line 50, in _rollback_on_error
    return function(self, *args, **kwargs)
DatabaseError: file is encrypted or is not a database

The proposal is to drop support for SQLite2 format and display message with a link to convert instructions when an old format is detected. ​http://trac.edgewall.org/wiki/PySqlite#TheSQLitelibrary

See also Debian bug: ​http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=501338

[Christian Boos]

Well, that error simply means that you're not accessing a SQLite v3 database, this could be a SQLite v2 file, or anything else.

I'm not sure it's worth making a special case for this error (or we could as well do so for the other errors listed in PySqlite#CommonOops).

That being said, dropping the support for PySqlite 1.0.x was on my TODO list for 0.12. I think there's no need for a deprecation period, as hardly anyone would be using it for new installations or even upgrades.

[anatoly techtonik <techtonik@…>]

I agree that deprecation is not needed, but there are ​still many installations of Trac 0.9 and even 0.8, so some kind of transition is still needed.

I would like to create warning on Trac 0.11.x if the old format of SQLite DB is still in use.

[Christian Boos]

OK, I'll do that for the upcoming 0.11.6.

[Remy Blank]

A quick question: currently, the SQLite backend always reports that it supports the sqlite scheme, even if the bindings could not be imported. Shouldn't get_supported_schemes() check if the bindings are available, and only return the supported scheme if they are?

The MySQL backend does that check, the PostgreSQL doesn't.

[Remy Blank]

Support for PySqlite 1.0.x was removed in [8571].

[Christian Boos]

Replying to rblank: And deprecation warning for 0.11.6 added in [8573].

A quick question: currently, the SQLite backend always reports that it supports the sqlite scheme, even if the bindings could not be imported. Shouldn't get_supported_schemes() check if the bindings are available, and only return the supported scheme if they are?

He, and that's just another patch I had in preparation (for #8562), where I did something along the lines of what we do for the version control connectors. See on that ticket.

[Christian Boos]

Better reporting of installation issues related to PySqlite.

[Christian Boos]

On second thought, the patch is better placed here, as the #8562 ticket is a bit confusing.

[Remy Blank]

I was thinking much simpler:

trac/db/sqlite_backend.py

@@ -90 +90 @@
         self._version = None
 
     def get_supported_schemes(self):
-        return [('sqlite', 1)]
+        if have_pysqlite:
+            return [('sqlite', 1)]
+        return []
 
     def get_connection(self, path, log=None, params={}):
         if not self._version:

It's always the same error anyway: the scheme is not supported by the current configuration, which is always due to missing bindings. Are you sure the extra "complexity" is worth it?

[Remy Blank]

Oh, and BTW, you should check for priority < 0 outside of the loop, otherwise a backend reporting an error could "shadow" a working backend for the same scheme.

Do you want me to apply the patch, and propagate the changes to the other backends?

[Christian Boos]

Replying to rblank:

I was thinking much simpler: … It's always the same error anyway: the scheme is not supported by the current configuration, which is always due to missing bindings.

We could also report when some known bad SQLite/PySqlite version is detected. For example, we should do so for PySqlite versions < 1.0.7 and < 2.0.7, which have known issues with SQLite ≥ 3.3.3. Also, we could "blacklist" problematic versions (like 2.5.0) instead of having workaround code for it. For each of these case (or future ones), we could have an informative message.

Are you sure the extra "complexity" is worth it?

It's the simplest way I found to propagate the error message up. It's not that complex, and we reuse a pattern already used for the vc connectors, so nothing fundamentally new.

[Christian Boos]

new version of the patch, more elaborate version checks for PySqlite and simplified DatabaseManager._get_connector

[anatoly techtonik <techtonik@…>]

As this one is initially my bug, I would like to record my vision. Unfortunately, I wasn't able to provide a patch, because of #8647 that took quite a lot of my time, but here is what I've tried:

• Add IEnvironmentSetupParticipant to SQLiteConnector

Failed, because SQLiteConnector component is called after Environment component and the latter already uses SQL queries in participant.environment_needs_upgrade(db) calls.

10:16:02 PM Trac[env] ERROR: Exception caught while checking for upgrade: 
Traceback (most recent call last):
  File "m:\p\trac\trac-0.11dev\trac\env.py", line 586, in open_environment
    needs_upgrade = env.needs_upgrade()
  File "m:\p\trac\trac-0.11dev\trac\env.py", line 429, in needs_upgrade
    if participant.environment_needs_upgrade(db):
  File "m:\p\trac\trac-0.11dev\trac\env.py", line 500, in environment_needs_upgrade
    dbver = self.env.get_version(db)
  File "m:\p\trac\trac-0.11dev\trac\env.py", line 351, in get_version
    (initial and 'initial_' or ''))
  File "m:\p\trac\trac-0.11dev\trac\db\util.py", line 60, in execute
    return self.cursor.execute(sql)
...

Of course if DB format is wrong there is no way for cursor.execute() to return correctly.

• Next I thought that if Environment class is called first - it should call the DB backend to figure out if it needs upgrade.

Index: trac/env.py
===================================================================
--- trac/env.py	(revision 8555)
+++ trac/env.py	(working copy)
@@ -497,6 +497,8 @@
         self._update_sample_config()
 
     def environment_needs_upgrade(self, db):
+        if db.db_needs_upgrade():
+            raise TracError(_('Database needs upgrade'))
         dbver = self.env.get_version(db)
         if dbver == db_default.db_version:
             return False

• And while trying to figure out where to plug this db_needs_upgrade() I've stuck with #8647

What we need (IMHO) is basically a big warning message in 0.11.6 if user still uses SQLite 2.x database with a link to instructions how to install proper bindings and upgrade the DB (in Debian I thought to provide a script to do the upgrade semi-automatically).

Let me present the other scenario - what happens if a fresh Trac installation is supplied with old environment and SQLite 2.x bindings are not present. Then there will be exception mentioned in this ticket description. It should be intercepted and before it is passed further DB should be checked for an old format by reading first 48 bytes from file and matching them against ** This file contains an SQLite 2.1 database ** string. I.e. do not propose to install old SQLite bindings, but urge people to upgrade to newer version instead, listing actual bugs for older version if possible. That was my original idea how to make this properly.

[Remy Blank]

Updated patch, adds support for errors in the other backends.

[Remy Blank]

The patch above reworks attachment:sqlite-get_supported_schemes.2​ slightly, and adds support for the new way of enumerating connectors to the PostgreSQL and MySQL backends.

Ok to apply to 0.11-stable and merge to trunk? I suppose that is what was intended, as the patch was against 0.11-stable.

[Christian Boos]

Yes, I didn't test it, but looks good, so I'm OK if you apply it (yes, 0.11-stable was the target).

[Remy Blank]

Patch applied in [8582].

[anatoly techtonik <techtonik@…>]

What about more prominent message about the necessity to upgrade SQLite 2.x database? The one that will be visible to everyone and not only in Trac logs?

According to PySqlite page: "It should still be possible to use the old SQLite 2.8.x version: everything seems to work (except the search, see #2960) You should then use an equally old Pysqlite version, namely 1.0.1. But doing so is really not recommended, you'll get awful performances and frequent "database is locked" errors (see #7785, #3446)."

It seems that upgrading to SQLite 3.x is something people should have been done even before 0.11.x

[Christian Boos]

Replying to anatoly techtonik <techtonik@…>:

What about more prominent message about the necessity to upgrade SQLite 2.x database? The one that will be visible to everyone and not only in Trac logs?

Well, I think the deprecation warning in 0.11.6 and the removal in 0.12 is more than enough. But we should take care of saying a word about this in the 0.11.6 announcement as well.

According to PySqlite page:

I'll update that.

It seems that upgrading to SQLite 3.x is something people should have been done even before 0.11.x

Certainly ;-)