Remove support for obsolete DB bindings

[Remy Blank]

Support for pysqlite-1.0.x and pyPgSql is deprecated in 0.11 and should be removed in 0.12.

[Christian Boos]

We already had tickets for that! ;-)

#8539 and #8625…

I have the deprecation patch for 0.11.6 ready to go, is that OK for you or had you something more elaborate in mind?

trac/db/sqlite_backend.py

@@ -118 +118 @@
 
     def get_connection(self, path, log=None, params={}):
         if not self._version:
-            global sqlite_version_string
+            global sqlite_version_string, have_pysqlite
             self._version = get_pkginfo(sqlite).get(
                 'version', '%d.%d.%s' % sqlite.version_info)
             self.env.systeminfo.extend([('SQLite', sqlite_version_string),
                                         ('pysqlite', self._version)])
+            if have_pysqlite == 1:
+                self.log.warning("Support for SQLite v2 and PySqlite 1.0.x "
+                                 "will be dropped in version 0.12, see "
+                                 "http://trac.edgewall.org/wiki/"
+                                 "PySqlite#UpgradingSQLitefrom2.xto3.x")
         return SQLiteConnection(path, log, params)
 
     def init_db(cls, path, log=None, params={}):

[Remy Blank]

Oops, sorry. I was going through TracDev/ToDo and thought that I could probably handle that. There was no ticket number next to these items, and I didn't bother to check.

Your patch looks fine (except that you could remove the global declaration altogether, as long as the variables are not written to, they will be found in the global scope anyway). Please apply.

I am working on 0.12, and have the SQLite part ready.

So, closing as a duplicate of #8539 and #8625.

[Christian Boos]

Replying to rblank:

… you could remove the global declaration altogether, as long as the variables are not written to, they will be found in the global scope anyway

Yes, but it seems that elsewhere in the code we use that as a hint to indicate where the variable comes from. I remember having been confused at least once when such a qualifier was not there, so I prefer to do it that way.

OTOH, I'm not against making a stricter usage of the keyword, if you think it's worth it, but then we should probably do it consistently and document it in the TracDev/CodingStyle as ​PEP:0008 doesn't say a word about this.

[Remy Blank]

Replying to cboos:

OTOH, I'm not against making a stricter usage of the keyword, if you think it's worth it, but then we should probably do it consistently and document it in the TracDev/CodingStyle as ​PEP:0008 doesn't say a word about this.

It's just the normal way of looking up names in Python. For example, every time you access a name that was imported from another module in the global scope, you do the same kind of lookup.

The global declaration is only needed if you want to write to a global variable from inside a function. From the Python docs:

The global statement is a declaration which holds for the entire current code block. It means that the listed identifiers are to be interpreted as globals. It would be impossible to assign to a global variable without global, although free variables may refer to globals without being declared global.

So I don't really think this needs to be documented…

[Christian Boos]

Replying to rblank:

Replying to cboos:

OTOH, I'm not against making a stricter usage of the keyword, if you think it's worth it, but then we should probably do it consistently and document it in the TracDev/CodingStyle as ​PEP:0008 doesn't say a word about this.

It's just the normal way of looking up names in Python. For example, every time you access a name that was imported from another module in the global scope, you do the same kind of lookup.

The global declaration is only needed if you want to write

I know, my point was that that using the "global" keyword as a hint that the variable we use (for read) is a global, seems to be an idiom in the Trac code (counted 19 occurrences). But we could change that.

[Remy Blank]

Replying to cboos:

I know, my point was that that using the "global" keyword as a hint that the variable we use (for read) is a global, seems to be an idiom in the Trac code (counted 19 occurrences).

That's not a lot, considering the actual number of accesses to global variables we do ;-) My feeling is that there were a few "legitimate" instances to start with, and this was misunderstood as an idiom.

Counting actual instances, I see 10 legitimate uses of global in 0.11-stable, versus 9 non-necessary uses. Not really what you'd call an idiom, I would say.

Anyway, I'll remove the unnecessary global declarations in 0.12 and document the change.