Integrate functionality of TracMigratePlugin

[Ryan J Ollos]

Database is locked is one of the most frequent duplicates. For migrating databases we suggest using ​TracMigratePlugin. The plugin is stable and I've used it for several migrations. We could move the code into tracopt. I'm seeking feedback on that idea. Jun, are you okay with the code being moved into the Trac core?

We might consider naming the module convert.py rather than migrate.py. The TracMigrate page documents a slightly different scenario.

[Ryan J Ollos]

I did some initial work in log:rjollos.git:t12299_trac_migrate. More work would be needed to move ahead.

[Jun Omae]

Jun, are you okay with the code being moved into the Trac core?

Yeah. No problem.

We could move the code into tracopt. …. We might consider naming the module convert.py rather than migrate.py.

It would be good to move the code into trac.admin.convert or trac.db.convert. I think it would be useful to be able to use without changes of [components] section.

[Ryan J Ollos]

Replying to jomae:

It would be good to move the code into trac.admin.convert or trac.db.convert. I think it would be useful to be able to use without changes of [components] section.

Those changes sound good.

I haven't been able to get the ​TracMigratePlugin tests to pass with PostgreSQL. After the following change they pass:

tracmigrate/tests/admin.py

@@ -67 +67 @@
         att.insert('filename.txt', StringIO('test'), 4)
         env.shutdown()
 
-    if 'destroying' in inspect.getargspec(EnvironmentStub.__init__)[0]:
-        def _destroy_db(self):
-            EnvironmentStub(destroying=True)
-    else:
-        def _destroy_db(self):
-            EnvironmentStub().destroy_db()
+    # if 'destroying' in inspect.getargspec(EnvironmentStub.__init__)[0]:
+    #     def _destroy_db(self):
+    #         EnvironmentStub(destroying=True)
+    # else:
+    def _destroy_db(self):
+        EnvironmentStub(destroying=True).destroy_db()
 
     def _get_all_records(self, env):
         def primary(row, columns):

[Jun Omae]

Replying to Ryan J Ollos:

I haven't been able to get the ​TracMigratePlugin tests to pass with PostgreSQL. After the following change they pass: […]

Thanks for the investigating! Fixed in ​th:r15147. I didn't probably test with Trac 1.0+ and PostgreSQL.

[Ryan J Ollos]

There are some changes we should incorporate in ​th:#12696, ​th:#12697 and ​th:#12691.

[Ryan J Ollos]

DONE Document obsolescence of ​TracMigratePlugin at 1.3/TracUpgrade.

[Ryan J Ollos]

For inplace migration, TracMigrate ​creates a temporary environment and for the SQLite case it ​copies the database file back to the migrated environment after the migration is finished. Would it work just as well if the source and destination environments were the same for an inplace migration?

[Ryan J Ollos]

I'm looking at moving the database conversion methods to trac.db.convert.DatabaseConverter, and putting the migrate command in trac.env.EnvironmentAdmin. That would result in two trac-admin commands:

• convertdb <dburi>: convert database (inplace)
• migrate <dstenv> [dburi]: migrate environment and optionally convert the database

[Emmanuel Blot]

I recently (and repeatedly) bumped into an issue with ​TracMigratePlugin and inherited trac.ini files:

Missing configuration file appkit/conf/trac.ini
Missing configuration file appkit/conf/site.ini
Missing configuration file appkit/conf/site.ini

I'm not sure what's going on here. Moreover I'm not sure yet if it tied but as the plugins are enabled in one of the inherited files, the DB tables for those plugins are not exported.

[Jun Omae]

Replying to Emmanuel Blot:

I recently (and repeatedly) bumped into an issue with ​TracMigratePlugin and inherited trac.ini files:

[…]

I'm not sure what's going on here. Moreover I'm not sure yet if it tied but as the plugins are enabled in one of the inherited files, the DB tables for those plugins are not exported.

That may be caused by non-absolute path in [inherit] file option. Could you please create new ticket with the details on ​trac-hacks.org?

[Christian Boos]

Moving to some later 1.3.x version.

[Ryan J Ollos]

After additional refactoring, proposed changes can be found in log:rjollos.git:t12299_trac_migrate.1.

There was a ​test failure, fixed by [b78163a2/rjollos.git]. That may also need to be fixed in ​TracMigratePlugin.

[Peter Suter]

Sounds great!

• Was tracopt/db/migrate.py supposed to be deleted as well in changeset:15d7c62a/rjollos.git?
• I have never used the the plugin. From the command name migrate (and its command help text) it was not immediately clear to me what its purpose is.
  • (For some reason I associate the term "migration" more with "DB schema upgrade".)
  • Is it mainly intended for migrating from one database backend (e.g. SQLite) to another (e.g. PostgreSQL)? (As described on wiki:DatabaseBackend#DatabaseConversion.)
  • Or also for migrating from older to newer database backend versions (e.g. from Pg8.0 to Pg9.6)?
  • Or also for migrating from one machine to another (e.g. when buying new server hardware)?
  • Should it also be used just to move the environment from one directory to another? (Moving a SQLite database with it, but leaving a PostgreSQL or MySQL database unaffected.)
  • The migrate command help text​ might need a grammar and clarity check.
  • If applicable maybe mention the word "database backend"?
  • If applicable maybe point out that the desired backend is selected / deduced from the db connection uri?
  • But maybe first decide about the final command names (comment:9)? convertdb sounds like an improvement to me.

[Ryan J Ollos]

Replying to Peter Suter:

Sounds great!

• Was tracopt/db/migrate.py supposed to be deleted as well in changeset:15d7c62a/rjollos.git?

Yes, the code is contained in trac/env.py and trac/db/convert.py. I also removed the trac-migrate.py script because I didn't see any value over the TracAdmin command.

The command name convertdb sounds good to me as well, but I welcome additional suggestions.

[Ryan J Ollos]

Replying to Peter Suter:

• (For some reason I associate the term "migration" more with "DB schema upgrade".)

There's good basis for associating migration with schema upgrade, for example ​Django migrations. From grepping the codebase I see we even use the term migration in some upgrade scripts and our database API, where the term is associated with database schema changes.

[Ryan J Ollos]

Changed to command name convert_db in [7912475e5/rjollos.git].

I'm considering changing signature from <tracenv|-i|--in-place> <dburi> to <dburi> [newenv]. The command forms would then be:

• trac-admin $env convert_db <dburi>
• trac-admin $env convert_db <dburi> </path/to/newenv>

On the one hand it's good to be explicit about wanting to do a an in-place conversion, which the -i/--in-place switch provides. However, since the focus of the command is now correctly on the "conversion", the latter forms feel more natural to me.

[Ryan J Ollos]

Replying to Ryan J Ollos:

I'm considering changing signature from <tracenv|-i|--in-place> <dburi> to <dburi> [newenv].

I've made those changes in log:rjollos.git:t12299_trac_migrate.3. Tests are passing, but I haven't done manual testing since making those changes. In absence of additional feedback, I'll do additional testing and then commit later in the week.

[Ryan J Ollos]

Committed to trunk in r16024.

[Ryan J Ollos]

Documented in 1.3/TracUpgrade@22.

[Ryan J Ollos]

Python3 compatible revision of r16024 in r16611.