From a8af87bd093bb4e37cf5850b9af45836fb9e47ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Moises=20Trov=C3=B3?= Date: Wed, 16 Sep 2015 17:46:36 -0300 Subject: [PATCH] Fix for sqlite support on migrations SQLite support was broken by a migration which needed an ALTER TABLE on a constraint of test_runs as ALTER TABLE support on SQLite is very shallow. Alembic developers suggest using batch_alter_table method for these situations http://alembic.readthedocs.org/en/latest/batch.html. Besides the batch_alter_table change a forced drop_index is needed for sqlite provider as index are created on a global namespace and it couldn't create a new temp table with an index with the same_ name. Change-Id: I287bc710369325bc626b62aea7eb6d0f46f8314a --- subunit2sql/migrations/env.py | 5 ++++- .../13d819bbb0ff_create_missing_indexes.py | 16 +++++++++++--- subunit2sql/tests/db_test_utils.py | 5 +++++ .../tests/migrations/test_migrations.py | 11 ++++++++++ subunit2sql/tests/subunit2sql_fixtures.py | 21 +++++++++++++++++++ 5 files changed, 54 insertions(+), 4 deletions(-) diff --git a/subunit2sql/migrations/env.py b/subunit2sql/migrations/env.py index 4ec91bd..4095a82 100644 --- a/subunit2sql/migrations/env.py +++ b/subunit2sql/migrations/env.py @@ -61,6 +61,7 @@ def run_migrations_offline(): else: kwargs['url'] = config.get_main_option("sqlalchemy.url") kwargs['target_metadata'] = target_metadata + kwargs['render_as_batch'] = True context.configure(**kwargs) with context.begin_transaction(): @@ -80,7 +81,9 @@ def run_migrations_online(): connection = engine.connect() facade._session_maker.configure(bind=connection) - context.configure(connection=connection, target_metadata=target_metadata) + context.configure(connection=connection, + target_metadata=target_metadata, + render_as_batch=True) try: with context.begin_transaction(): diff --git a/subunit2sql/migrations/versions/13d819bbb0ff_create_missing_indexes.py b/subunit2sql/migrations/versions/13d819bbb0ff_create_missing_indexes.py index 624aa85..60f2f28 100644 --- a/subunit2sql/migrations/versions/13d819bbb0ff_create_missing_indexes.py +++ b/subunit2sql/migrations/versions/13d819bbb0ff_create_missing_indexes.py @@ -43,6 +43,19 @@ def upgrade(): if ('ix_test_id' not in test_indx_names and 'test_id' not in test_indx_columns): op.create_index('ix_test_id', 'tests', ['test_id'], mysql_length=30) + + # remove auto created indexes (sqlite only) + # note the name is with test_runs not test_run + if migration_context.dialect.name == 'sqlite': + if 'ix_test_runs_test_id' in test_run_indx_names: + op.drop_index('ix_test_runs_test_id', 'test_runs') + if 'ix_test_runs_run_id' in test_run_indx_names: + op.drop_index('ix_test_runs_run_id', 'test_runs') + + with op.batch_alter_table('test_runs') as batch_op: + batch_op.create_unique_constraint('uq_test_runs', + ['test_id', 'run_id']) + if ('ix_test_run_test_id' not in test_run_indx_names and 'test_id' not in test_run_indx_columns): op.create_index('ix_test_run_test_id', 'test_runs', ['test_id']) @@ -50,9 +63,6 @@ def upgrade(): 'run_id' not in test_run_indx_columns): op.create_index('ix_test_run_run_id', 'test_runs', ['run_id']) - op.create_unique_constraint('uq_test_runs', 'test_runs', - ['test_id', 'run_id']) - def downgrade(): op.drop_constraint('uq_test_runs', 'test_runs') diff --git a/subunit2sql/tests/db_test_utils.py b/subunit2sql/tests/db_test_utils.py index 673606c..c621f35 100644 --- a/subunit2sql/tests/db_test_utils.py +++ b/subunit2sql/tests/db_test_utils.py @@ -13,6 +13,7 @@ # under the License. import os +import tempfile from alembic import command from alembic import config as alembic_config @@ -22,6 +23,7 @@ import sqlalchemy from subunit2sql.db import api as session CONF = cfg.CONF +SQLITE_TEST_DATABASE_PATH = tempfile.mkstemp('subunit2sql.db')[1] script_location = os.path.join(os.path.dirname(os.path.dirname( os.path.abspath(__file__))), 'migrations') @@ -41,6 +43,9 @@ def get_connect_string(backend, elif backend == "postgres": backend = "postgresql+psycopg2" + if backend == "sqlite": + return "sqlite:///" + SQLITE_TEST_DATABASE_PATH + return ("%(backend)s://%(user)s:%(passwd)s@localhost/%(database)s" % {'backend': backend, 'user': user, 'passwd': passwd, 'database': database}) diff --git a/subunit2sql/tests/migrations/test_migrations.py b/subunit2sql/tests/migrations/test_migrations.py index 216174d..b0f6377 100644 --- a/subunit2sql/tests/migrations/test_migrations.py +++ b/subunit2sql/tests/migrations/test_migrations.py @@ -194,6 +194,17 @@ class TestWalkMigrations(base.TestCase): # build a fully populated postgresql database with all the tables self._walk_versions(engine) + def test_sqlite_opportunistically(self): + self.useFixture(fixtures.LockFixture('sqlite')) + self.useFixture(fixtures.SqliteConfFixture()) + + connect_string = db_test_utils.get_connect_string("sqlite") + engine = sqlalchemy.create_engine(connect_string) + self.engines["sqlitecitest"] = engine + self.test_databases["sqlitecitest"] = connect_string + + self._walk_versions(engine) + def _pre_upgrade_1f92cfe8a6d3(self, engine): tests = get_table(engine, 'tests') data = {'id': 'fake_test.id', diff --git a/subunit2sql/tests/subunit2sql_fixtures.py b/subunit2sql/tests/subunit2sql_fixtures.py index 907158c..295b3f8 100644 --- a/subunit2sql/tests/subunit2sql_fixtures.py +++ b/subunit2sql/tests/subunit2sql_fixtures.py @@ -140,6 +140,27 @@ class PostgresConfFixture(config_fixture.Config): self._drop_db() +class SqliteConfFixture(config_fixture.Config): + """Fixture to manage global conf settings.""" + def _drop_db(self): + if os.path.exists(db_test_utils.SQLITE_TEST_DATABASE_PATH): + os.remove(db_test_utils.SQLITE_TEST_DATABASE_PATH) + + def setUp(self): + super(SqliteConfFixture, self).setUp() + self.register_opts(options.database_opts, group='database') + self.register_opts(cli.MIGRATION_OPTS) + self.url = db_test_utils.get_connect_string("sqlite") + self.set_default('connection', self.url, group='database') + self.set_default('disable_microsecond_data_migration', False) + lockutils.set_defaults(lock_path='/tmp') + self._drop_db() + self.addCleanup(self.cleanup) + + def cleanup(self): + self._drop_db() + + class LockFixture(lock_fixture.LockFixture): def __init__(self, name): lockutils.set_defaults(lock_path='/tmp')