#8172 closed enhancement (fixed)
Plugin db upgrade infrastructure — at Version 28
Reported by: | Owned by: | Peter Suter | |
---|---|---|---|
Priority: | low | Milestone: | 1.1.5 |
Component: | general | Version: | none |
Severity: | minor | Keywords: | |
Cc: | osimons, Peter Suter, Ryan J Ollos, olemis@…, olemis+trac@… | Branch: | |
Release Notes: |
Added infrastructure for plugins to upgrade the database. |
||
API Changes: |
Added |
||
Internal Changes: |
Description
Trac currently has its own infrastructure to manage db upgrades. However for plugin authors there is no mechanism available for that so every plugin author needs to re-invent this again.
Attached is a module which basically uses trac's mechanism but is a bit more generic so this could be used by plugins as well.
Change History (30)
by , 15 years ago
Attachment: | plugin_env_setup.py added |
---|
comment:2 by , 15 years ago
Hmm, I hoped we could have this in 0.12… Do you think this feature could make it?
comment:3 by , 15 years ago
About milestones, 0.12 means a developer is committed to integrating the patch in trunk. 0.13 means a developer is committed to integrating the patch, but in the next version. Everything beyond that means we would like to do it eventually, but nobody has taken it up yet.
Personally, I don't have enough knowledge about this part of Trac that I could review and integrate your patch. Sorry about that. If you can persuade another dev to take this up, he will move the milestone closer as well, possibly to 0.12.
comment:5 by , 14 years ago
Milestone: | triaging → 0.13 |
---|---|
Owner: | set to |
I'd like to pick this up. However, rather than having an abstract component from which plugins would inherit, how about creating a new IDBUpgradeParticipant
interface that would be implemented in plugins and provide the plugin name, the expected DB version, possibly the package containing the upgrade scripts, as well as a method for the initial DB setup? Then a single component could implement the upgrade logic, and be used by core as well.
comment:6 by , 14 years ago
I also think we don't need to inherit from an abstract component here.
However, I'm not sure if it's worth to introduce a new interface, we could perhaps do as well with a set of helper methods (factoring out the code already in env.py and mixing it with some parts of the proposed patch).
comment:7 by , 14 years ago
… and yes, this would not only be useful for plugins, but also for core sub-systems, to get finer grained upgrades (see #9222).
follow-up: 10 comment:8 by , 14 years ago
#9222… Well, this is back to the core vs plugins vs future of Trac development…
Personally I don't see the need for new interfaces either. I think the IEnvironmentSetupParticipant
interface is perfectly capable of handling all kinds of upgrades. What is missing is, as also mentioned in #9222, moving upgrades into the various components parts of Trac and letting each self-contained part of Trac be responsible for its own upgrades.
This was the original vision of the component code before things got more and more 'tangled' into each other. For a remaining example, see for instance how attachment.py
implements the interface for the sole purpose of creating an 'attachments' folder for new projects.
Naturally, the Trac core code can improve its (optional) services/functions/helpers for module and plugin upgrades - just don't invent a whole new infrastructure for this that will never be able to cover all needs anyway, IMHO.
comment:9 by , 14 years ago
Cc: | added |
---|
comment:10 by , 14 years ago
Replying to osimons:
just don't invent a whole new infrastructure for this that will never be able to cover all needs anyway, IMHO.
That's a bit pessimistic, but I think you (and Christian) are right. Let's try to add a few helper functions to more easily allow distributing upgrades into individual modules and plugins.
That's one risk when you have a cool component architecture: you tend to use it too much…
follow-up: 14 comment:11 by , 13 years ago
Would the helper functions in the patch attached above be a step in the right direction?
They mostly refactor out functionality from Environment
and EnvironmentSetup
for DB related environment setup (schema setup, initial data insertion, version check and per-version upgrades).
A plugin could quite easily reuse them to manage its DB schema:
from trac.core import * from trac.env import * from trac.db.schema import Table, Column PLUGIN_NAME = 'ExamplePlugin' PLUGIN_VERSION = 2 PLUGIN_SCHEMA = [ Table('table1', key='name')[ Column('name'), Column('value')]] def get_initial_plugin_data(db): return (('table1', ('name', 'value'), (('name1', 'value1'), ('name2', 'value2'))),) class ExamplePlugin(Component): implements(IEnvironmentSetupParticipant) # IEnvironmentSetupParticipant methods def environment_created(self): init_environment_schema_for_module(self.env, PLUGIN_SCHEMA) init_environment_data_for_module(self.env, get_initial_plugin_data) init_environment_version_for_module(self.env, PLUGIN_NAME, PLUGIN_VERSION) def environment_needs_upgrade(self, db): return environment_needs_upgrade_for_module(self.env, PLUGIN_NAME, PLUGIN_NAME, PLUGIN_VERSION) def upgrade_environment(self, db): upgrade_environment_for_module(self.env, PLUGIN_NAME, PLUGIN_NAME, PLUGIN_VERSION, 'ExamplePlugin_upgrades')
(It could also freely mix and match these functions with its own code, or ignore them altogether.)
I'm not too sure with the names. Maybe it would be good to drop or replace the for_module
suffix, include _db_
somewhere, or even move them to trac.db.util
?
What do you think?
comment:12 by , 12 years ago
Cc: | added |
---|
follow-ups: 15 18 comment:14 by , 10 years ago
I still like this idea with more helpers functions.
Replying to psuter:
I'm not too sure with the names. Maybe it would be good to drop or replace the
for_module
suffix, include_db_
somewhere, or even move them totrac.db.util
?
Maybe they should be methods on Environment
or DatabaseManager
.
The proposed init_environment_schema_for_module
is actually almost identical to the new DatabaseManager.create_tables(schema)
added in #11512.
And the proposed get_environment_version_for_module
is very similar to Environment.get_version()
. (Although that was deprecated(?) in #11681.)
I think we could instead add more useful methods:
Environment.get_database_version(name='database_version')
Environment.set_database_version(name, version)
Environment.needs_database_upgrade(name, version)
Environment.run_database_upgrades(name, version, upgrades_pkg)
DatabaseManager.insert_data(data)
(optional:data
is callable to receivedb
?)
The implementations would be similar to the functions in the patch.
The example plugin would look like this:
from trac.core import * from trac.env import * from trac.db.schema import Table, Column PLUGIN_NAME = 'ExamplePlugin' PLUGIN_VERSION = 2 PLUGIN_SCHEMA = [ Table('table1', key='name')[ Column('name'), Column('value')]] INITIAL_PLUGIN_DATA = ( ('table1', ('name', 'value'), (('name1', 'value1'), ('name2', 'value2'))),) class ExamplePlugin(Component): implements(IEnvironmentSetupParticipant) # IEnvironmentSetupParticipant methods def environment_created(self): DatabaseManager(self).create_tables(PLUGIN_SCHEMA) DatabaseManager(self).insert_data(INITIAL_PLUGIN_DATA) self.env.set_database_version(PLUGIN_NAME, PLUGIN_VERSION) def environment_needs_upgrade(self, db): return self.env.needs_database_upgrade(PLUGIN_NAME, PLUGIN_VERSION) def upgrade_environment(self, db): self.env.run_database_upgrades(PLUGIN_NAME, PLUGIN_VERSION)
follow-up: 16 comment:15 by , 10 years ago
Cc: | added |
---|
Replying to psuter:
I still like this idea with more helpers functions.
Same here. It's been on my mind for a while and I wasn't aware of this ticket. Sounds like there are some good ideas here.
Maybe they should be methods on
Environment
orDatabaseManager
.
That seems like the right way to go, maybe even have all the helpers exist as methods of the DatabaseManager
class if that works.
The proposed
init_environment_schema_for_module
is actually almost identical to the newDatabaseManager.create_tables(schema)
added in #11512.And the proposed
get_environment_version_for_module
is very similar toEnvironment.get_version()
. (Although that was deprecated(?) in #11681.)
Deprecated as a public method in favor of the new properties Environment.database_version
and Environment.database_initial_version
. Effectively only the calling syntax has changed. The return value is the same. So Environment.get_version()
is replaced by Environment.database_version
and Environment.get_version(initial=True)
is replaced by Environment.database_initial_version
. The properties avoid a database hit on every access.
I think we could instead add more useful methods:
Environment.get_database_version(name='database_version')
Environment.set_database_version(name, version)
get_schema_version
and set_schema_version
might be good names as well, however since the upgrades can change the database version without modifying the schema, database_version
might be better. Having the methods exist on the DatabaseManager
class could avoid ambiguity with database_version
of the Environment
class.
Environment.needs_database_upgrade(name, version)
Environment.run_database_upgrades(name, version, upgrades_pkg)
DatabaseManager.insert_data(data)
(optional:data
is callable to receivedb
?)
insert_data
might be useful in Trac unit tests as well.
comment:16 by , 9 years ago
Replying to rjollos:
I think we could instead add more useful methods:
Environment.get_database_version(name='database_version')
Environment.set_database_version(name, version)
get_schema_version
andset_schema_version
might be good names as well, however since the upgrades can change the database version without modifying the schema,database_version
might be better. Having the methods exist on theDatabaseManager
class could avoid ambiguity withdatabase_version
of theEnvironment
class.
get_database_version
and set_database_version
are proposed in #11859.
comment:17 by , 9 years ago
Milestone: | next-major-releases → next-dev-1.1.x |
---|
comment:18 by , 9 years ago
Replying to psuter:
DatabaseManager.insert_data(data)
(optional:data
is callable to receivedb
?)
Adding populate_tables
method to DatabaseManager
is proposed in #11893. I'm okay with the name insert_data
as well. I chose populate_tables
for symmetry with create_tables
, drop_tables
and reset_tables
.
comment:22 by , 9 years ago
Cc: | added |
---|
by , 9 years ago
Attachment: | T8172-upgrade-helpers.diff added |
---|
Extract DatabaseManager.upgrade()
and DatabaseManager.needs_upgrade()
follow-up: 24 comment:23 by , 9 years ago
After #11893 (DatabaseManager.insert_into_tables()
) and #11859 (DatabaseManager.get_database_version()
and DatabaseManager.set_database_version()
) the main missing functionality is running upgrade scripts. In the attached patch I propose extracting DatabaseManager.upgrade()
and DatabaseManager.needs_upgrade()
.
Example plugin:
example/
upgrades
__init__.py
(empty)db2.py
def do_upgrade(env, ver, cursor): pass
__init__.py
(empty)core.py
from trac.core import * from trac.env import * from trac.db.api import DatabaseManager from trac.db.schema import Table, Column PLUGIN_NAME = 'ExamplePlugin' PLUGIN_VERSION = 2 PLUGIN_SCHEMA = [ Table('table1', key='name')[ Column('name'), Column('value')]] INITIAL_PLUGIN_DATA = ( ('table1', ('name', 'value'), (('name1', 'value1'), ('name2', 'value2'))),) class ExamplePlugin(Component): implements(IEnvironmentSetupParticipant) # IEnvironmentSetupParticipant methods def environment_created(self): dbm = DatabaseManager(self.env) dbm.create_tables(PLUGIN_SCHEMA) dbm.insert_into_tables(INITIAL_PLUGIN_DATA) dbm.set_database_version(PLUGIN_VERSION, PLUGIN_NAME) def environment_needs_upgrade(self, db): dbm = DatabaseManager(self.env) return dbm.needs_upgrade(PLUGIN_VERSION, PLUGIN_NAME) def upgrade_environment(self, db): dbm = DatabaseManager(self.env) if dbm.get_database_version(PLUGIN_NAME) == 0: dbm.create_tables(PLUGIN_SCHEMA) dbm.insert_into_tables(INITIAL_PLUGIN_DATA) dbm.set_database_version(PLUGIN_VERSION, PLUGIN_NAME) else: dbm.upgrade(PLUGIN_VERSION, PLUGIN_NAME, 'example.upgrades')
setup.py
#!/usr/bin/env python from setuptools import setup setup(name = 'example', version = '1.0', packages = ['example', 'example.upgrades'], entry_points = {'trac.plugins': ['example.core = example.core']})
comment:24 by , 9 years ago
Replying to psuter:
After #11893 (
DatabaseManager.insert_into_tables()
) and #11859 (DatabaseManager.get_database_version()
andDatabaseManager.set_database_version()
) the main missing functionality is running upgrade scripts. In the attached patch I propose extractingDatabaseManager.upgrade()
andDatabaseManager.needs_upgrade()
.
I think the pkg
argument of DatabaseManager.upgrade()
should have None
for the default value. If plugin author neglects to specify own package to it, that would lead pretty bad.
comment:25 by , 9 years ago
Besides issue noted in comment:24, looks good to me. I did some minor refactoring in log:rjollos.git:t8172. Test coverage could be added to DatabaseManagerTestCase.
comment:26 by , 9 years ago
Regarding ExamplePlugin
in comment:23, you may already be away of these, but in case not:
- The
db
arguments ofenvironment_needs_upgrade
andupgrade_environment
are optional and deprecated. environment_created
could just callupgrade_environment
.
comment:27 by , 9 years ago
Milestone: | next-dev-1.1.x → 1.1.5 |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Thanks for the reviews!
comment:28 by , 9 years ago
API Changes: | modified (diff) |
---|---|
Release Notes: | modified (diff) |
Resolution: | → fixed |
Status: | assigned → closed |
Committed in [13926].
If there is a chance to get this into trac main, we can polish this class (including docs) and make trac use it too