From f9bc7715d607749a9399186ae5325b193bd97903 Mon Sep 17 00:00:00 2001 From: Aniruddha Maru Date: Wed, 6 May 2020 21:42:20 -0700 Subject: [PATCH 1/3] Add new comment: show migrations --- README.md | 3 +++ spanner_orm/admin/migration_executor.py | 14 +++++++++++++- spanner_orm/admin/scripts.py | 17 +++++++++++++++-- spanner_orm/tests/migrations_test.py | 22 +++++++++++++++++++++- 4 files changed, 52 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index c3d9a02..ce54fc3 100644 --- a/README.md +++ b/README.md @@ -175,3 +175,6 @@ multiple times, so try not to do that. If a migration needs to be rolled back, ```spanner-orm rollback ``` or the corresponding ```MigrationExecutor``` method should be used. + +To see a list of all migrations found, run ```spanner-orm showmigrations ```. +Migrations that have already been applied migrations are marked by an `[X]`. diff --git a/spanner_orm/admin/migration_executor.py b/spanner_orm/admin/migration_executor.py index 1d9e9d5..dc64a0b 100644 --- a/spanner_orm/admin/migration_executor.py +++ b/spanner_orm/admin/migration_executor.py @@ -76,6 +76,18 @@ def migrate(self, target_migration: Optional[str] = None) -> None: self._update_status(migration_.migration_id, True) self._hangup() + def show_migrations(self) -> None: + """Prints information about all migrations. + """ + self._connect() + self._validate_migrations() + + for migration_ in reversed(self.migrations()): + migrated = self.migrated(migration_.migration_id) + _logger.info('[%s] Migration %s', 'X' if migrated else ' ', migration_.migration_id) + + self._hangup() + def rollback(self, target_migration: str) -> None: """Rolls back migrated migrations on the curent database. @@ -117,7 +129,7 @@ def _hangup(self) -> None: def _filter_migrations( self, migrations: Iterable[migration.Migration], migrated: bool, - last_migration: Optional[str]) -> List[migration.Migration]: + last_migration: Optional[str] = None) -> List[migration.Migration]: """Filters the list of migrations according to the desired conditions. Args: diff --git a/spanner_orm/admin/scripts.py b/spanner_orm/admin/scripts.py index dd445f9..c3f387e 100644 --- a/spanner_orm/admin/scripts.py +++ b/spanner_orm/admin/scripts.py @@ -33,6 +33,12 @@ def migrate(args: Any) -> None: executor.migrate(args.name) +def show_migrations(args: Any) -> None: + connection = api.SpannerConnection(args.instance, args.database) + executor = migration_executor.MigrationExecutor(connection, args.directory) + executor.show_migrations() + + def rollback(args: Any) -> None: connection = api.SpannerConnection(args.instance, args.database) executor = migration_executor.MigrationExecutor(connection, args.directory) @@ -56,14 +62,21 @@ def main(as_module: bool = False) -> None: generate_parser.set_defaults(execute=generate) migrate_parser = subparsers.add_parser( - 'migrate', help='Execute unmigrated migrations') + 'migrate', help='Execute unmigrated migrations') migrate_parser.add_argument( - '--name', help='Stop migrating after this migration') + '--name', help='Stop migrating after this migration') migrate_parser.add_argument('--directory') migrate_parser.add_argument('instance', help='Name of Spanner instance') migrate_parser.add_argument('database', help='Name of Spanner database') migrate_parser.set_defaults(execute=migrate) + show_migrations_parser = subparsers.add_parser( + 'showmigrations', help='List migrations') + show_migrations_parser.add_argument('--directory') + show_migrations_parser.add_argument('instance', help='Name of Spanner instance') + show_migrations_parser.add_argument('database', help='Name of Spanner database') + show_migrations_parser.set_defaults(execute=show_migrations) + rollback_parser = subparsers.add_parser( 'rollback', help='Roll back migrated migrations') rollback_parser.add_argument( diff --git a/spanner_orm/tests/migrations_test.py b/spanner_orm/tests/migrations_test.py index 1324e07..3379ba6 100644 --- a/spanner_orm/tests/migrations_test.py +++ b/spanner_orm/tests/migrations_test.py @@ -225,7 +225,7 @@ def test_validate_migrations_error_on_unmigrated_first(self): def test_migrate(self): connection = mock.Mock() executor = migration_executor.MigrationExecutor( - connection, self.TEST_MIGRATIONS_DIR) + connection, self.TEST_MIGRATIONS_DIR) first = migration.Migration('1', None) second = migration.Migration('2', '1') @@ -237,6 +237,26 @@ def test_migrate(self): executor.migrate() self.assertEqual(migrated, {'1': True, '2': True, '3': True}) + def test_showmigrations(self): + connection = mock.Mock() + executor = migration_executor.MigrationExecutor( + connection, self.TEST_MIGRATIONS_DIR) + + first = migration.Migration('1', None) + second = migration.Migration('2', '1') + third = migration.Migration('3', '2') + with mock.patch.object(executor, 'migrations') as migrations: + migrations.return_value = [first, second, third] + migrated = {'1': True, '2': False, '3': False} + with mock.patch.object(executor, '_migration_status_map', migrated): + with mock.patch('spanner_orm.admin.migration_executor._logger.info') as mock_logging: + executor.show_migrations() + mock_logging.assert_has_calls([ + mock.call('[%s] Migration %s', ' ', '3'), + mock.call('[%s] Migration %s', ' ', '2'), + mock.call('[%s] Migration %s', 'X', '1'), + ]) + def test_rollback(self): connection = mock.Mock() executor = migration_executor.MigrationExecutor( From 0f58dcb598d514dd4e6f44c542dabd36469555b0 Mon Sep 17 00:00:00 2001 From: Aniruddha Maru Date: Wed, 6 May 2020 22:12:18 -0700 Subject: [PATCH 2/3] print --- spanner_orm/admin/migration_executor.py | 2 +- spanner_orm/tests/migrations_test.py | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/spanner_orm/admin/migration_executor.py b/spanner_orm/admin/migration_executor.py index dc64a0b..203ce0d 100644 --- a/spanner_orm/admin/migration_executor.py +++ b/spanner_orm/admin/migration_executor.py @@ -84,7 +84,7 @@ def show_migrations(self) -> None: for migration_ in reversed(self.migrations()): migrated = self.migrated(migration_.migration_id) - _logger.info('[%s] Migration %s', 'X' if migrated else ' ', migration_.migration_id) + print('[{}] Migration {}'.format('X' if migrated else ' ', migration_.migration_id)) self._hangup() diff --git a/spanner_orm/tests/migrations_test.py b/spanner_orm/tests/migrations_test.py index 3379ba6..a81bb5b 100644 --- a/spanner_orm/tests/migrations_test.py +++ b/spanner_orm/tests/migrations_test.py @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from io import StringIO import logging import os import shutil @@ -249,13 +250,9 @@ def test_showmigrations(self): migrations.return_value = [first, second, third] migrated = {'1': True, '2': False, '3': False} with mock.patch.object(executor, '_migration_status_map', migrated): - with mock.patch('spanner_orm.admin.migration_executor._logger.info') as mock_logging: + with mock.patch('sys.stdout', new_callable=StringIO) as mock_stdout: executor.show_migrations() - mock_logging.assert_has_calls([ - mock.call('[%s] Migration %s', ' ', '3'), - mock.call('[%s] Migration %s', ' ', '2'), - mock.call('[%s] Migration %s', 'X', '1'), - ]) + self.assertEqual("[ ] Migration 3\n[ ] Migration 2\n[X] Migration 1\n", mock_stdout.getvalue()) def test_rollback(self): connection = mock.Mock() From bdf09a53250327306aa7245586612a144d8ec3c3 Mon Sep 17 00:00:00 2001 From: Aniruddha Maru Date: Thu, 7 May 2020 09:06:28 -0700 Subject: [PATCH 3/3] Add migration doc string --- spanner_orm/admin/migration.py | 6 ++ spanner_orm/admin/migration_executor.py | 2 +- spanner_orm/admin/migration_manager.py | 6 ++ spanner_orm/tests/migrations_test.py | 84 ++++++++++++------------- 4 files changed, 55 insertions(+), 43 deletions(-) diff --git a/spanner_orm/admin/migration.py b/spanner_orm/admin/migration.py index 177e1a7..623033b 100644 --- a/spanner_orm/admin/migration.py +++ b/spanner_orm/admin/migration.py @@ -29,9 +29,11 @@ class Migration: def __init__(self, migration_id: str, prev_migration_id: Optional[str], + description: str, upgrade: Optional[Callable[[], update.SchemaUpdate]] = None, downgrade: Optional[Callable[[], update.SchemaUpdate]] = None): self._id = migration_id + self._description = description self._prev = prev_migration_id self._upgrade = upgrade or no_update_callable self._downgrade = downgrade or no_update_callable @@ -44,6 +46,10 @@ def migration_id(self) -> str: def prev_migration_id(self) -> Optional[str]: return self._prev + @property + def description(self) -> str: + return self._description + @property def upgrade(self) -> Optional[Callable[[], update.SchemaUpdate]]: return self._upgrade diff --git a/spanner_orm/admin/migration_executor.py b/spanner_orm/admin/migration_executor.py index 203ce0d..54a9980 100644 --- a/spanner_orm/admin/migration_executor.py +++ b/spanner_orm/admin/migration_executor.py @@ -84,7 +84,7 @@ def show_migrations(self) -> None: for migration_ in reversed(self.migrations()): migrated = self.migrated(migration_.migration_id) - print('[{}] Migration {}'.format('X' if migrated else ' ', migration_.migration_id)) + print('[{}] {}, {}'.format('X' if migrated else ' ', migration_.migration_id, migration_.description)) self._hangup() diff --git a/spanner_orm/admin/migration_manager.py b/spanner_orm/admin/migration_manager.py index da1a29f..ebab757 100644 --- a/spanner_orm/admin/migration_manager.py +++ b/spanner_orm/admin/migration_manager.py @@ -75,9 +75,15 @@ def _migration_from_file(self, filename: str) -> migration.Migration: spec = importlib.util.spec_from_file_location(module_name, path) module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) + module_doc = module.__doc__.split('\n') + if not module_doc: + description = "" + else: + description = module_doc[0] try: result = migration.Migration(module.migration_id, module.prev_migration_id, + description, getattr(module, 'upgrade', None), getattr(module, 'downgrade', None)) except AttributeError: diff --git a/spanner_orm/tests/migrations_test.py b/spanner_orm/tests/migrations_test.py index a81bb5b..1697184 100644 --- a/spanner_orm/tests/migrations_test.py +++ b/spanner_orm/tests/migrations_test.py @@ -67,9 +67,9 @@ def test_generate(self): shutil.rmtree(self.TEST_MIGRATIONS_DIR) def test_order_migrations(self): - first = migration.Migration('1', None) - second = migration.Migration('2', '1') - third = migration.Migration('3', '2') + first = migration.Migration('1', None, '1') + second = migration.Migration('2', '1', '2') + third = migration.Migration('3', '2', '3') migrations = [third, first, second] expected_order = [first, second, third] @@ -77,9 +77,9 @@ def test_order_migrations(self): self.assertEqual(manager._order_migrations(migrations), expected_order) def test_order_migrations_with_no_none(self): - first = migration.Migration('2', '1') - second = migration.Migration('3', '2') - third = migration.Migration('4', '3') + first = migration.Migration('2', '1', '2') + second = migration.Migration('3', '2', '3') + third = migration.Migration('4', '3', '4') migrations = [third, first, second] expected_order = [first, second, third] @@ -87,9 +87,9 @@ def test_order_migrations_with_no_none(self): self.assertEqual(manager._order_migrations(migrations), expected_order) def test_order_migrations_error_on_unclear_successor(self): - first = migration.Migration('1', None) - second = migration.Migration('2', '1') - third = migration.Migration('3', '1') + first = migration.Migration('1', None, '1') + second = migration.Migration('2', '1', '2') + third = migration.Migration('3', '1', '3') migrations = [third, first, second] manager = migration_manager.MigrationManager(self.TEST_MIGRATIONS_DIR) @@ -97,8 +97,8 @@ def test_order_migrations_error_on_unclear_successor(self): manager._order_migrations(migrations) def test_order_migrations_error_on_unclear_start_migration(self): - first = migration.Migration('1', None) - second = migration.Migration('3', '2') + first = migration.Migration('1', None, '1') + second = migration.Migration('3', '2', '3') migrations = [first, second] manager = migration_manager.MigrationManager(self.TEST_MIGRATIONS_DIR) @@ -106,9 +106,9 @@ def test_order_migrations_error_on_unclear_start_migration(self): manager._order_migrations(migrations) def test_order_migrations_error_on_circular_dependency(self): - first = migration.Migration('1', '3') - second = migration.Migration('2', '1') - third = migration.Migration('3', '2') + first = migration.Migration('1', '3', '1') + second = migration.Migration('2', '1', '2') + third = migration.Migration('3', '2', '3') migrations = [third, first, second] manager = migration_manager.MigrationManager(self.TEST_MIGRATIONS_DIR) @@ -116,9 +116,9 @@ def test_order_migrations_error_on_circular_dependency(self): manager._order_migrations(migrations) def test_order_migrations_error_on_no_successor(self): - first = migration.Migration('1', None) - second = migration.Migration('2', '3') - third = migration.Migration('3', '2') + first = migration.Migration('1', None, '1') + second = migration.Migration('2', '3', '2') + third = migration.Migration('3', '2', '3') migrations = [third, first, second] manager = migration_manager.MigrationManager(self.TEST_MIGRATIONS_DIR) @@ -130,9 +130,9 @@ def test_filter_migrations(self): executor = migration_executor.MigrationExecutor( connection, self.TEST_MIGRATIONS_DIR) - first = migration.Migration('1', None) - second = migration.Migration('2', '1') - third = migration.Migration('3', '2') + first = migration.Migration('1', None, '1') + second = migration.Migration('2', '1', '2') + third = migration.Migration('3', '2', '3') migrations = [first, second, third] migrated = {'1': True, '2': False, '3': False} @@ -151,9 +151,9 @@ def test_filter_migrations_error_on_bad_last_migration(self): executor = migration_executor.MigrationExecutor( connection, self.TEST_MIGRATIONS_DIR) - first = migration.Migration('1', None) - second = migration.Migration('2', '1') - third = migration.Migration('3', '2') + first = migration.Migration('1', None, '1') + second = migration.Migration('2', '1', '2') + third = migration.Migration('3', '2', '3') migrations = [first, second, third] migrated = {'1': True, '2': False, '3': False} @@ -169,9 +169,9 @@ def test_validate_migrations(self): executor = migration_executor.MigrationExecutor( connection, self.TEST_MIGRATIONS_DIR) - first = migration.Migration('1', None) - second = migration.Migration('2', '1') - third = migration.Migration('3', '2') + first = migration.Migration('1', None, '1') + second = migration.Migration('2', '1', '2') + third = migration.Migration('3', '2', '3') with mock.patch.object(executor, 'migrations') as migrations: migrations.return_value = [first, second, third] @@ -188,9 +188,9 @@ def test_validate_migrations_error_on_unmigrated_after_migrated(self): executor = migration_executor.MigrationExecutor( connection, self.TEST_MIGRATIONS_DIR) - first = migration.Migration('1', None) - second = migration.Migration('2', '1') - third = migration.Migration('3', '2') + first = migration.Migration('1', None, '1') + second = migration.Migration('2', '1', '2') + third = migration.Migration('3', '2', '3') with mock.patch.object(executor, 'migrations') as migrations: migrations.return_value = [first, second, third] @@ -209,7 +209,7 @@ def test_validate_migrations_error_on_unmigrated_first(self): executor = migration_executor.MigrationExecutor( connection, self.TEST_MIGRATIONS_DIR) - first = migration.Migration('2', '1') + first = migration.Migration('2', '1', '2') with mock.patch.object(executor, 'migrations') as migrations: migrations.return_value = [first] @@ -228,9 +228,9 @@ def test_migrate(self): executor = migration_executor.MigrationExecutor( connection, self.TEST_MIGRATIONS_DIR) - first = migration.Migration('1', None) - second = migration.Migration('2', '1') - third = migration.Migration('3', '2') + first = migration.Migration('1', None, '1') + second = migration.Migration('2', '1', '2') + third = migration.Migration('3', '2', '3') with mock.patch.object(executor, 'migrations') as migrations: migrations.return_value = [first, second, third] migrated = {'1': True, '2': False, '3': False} @@ -238,30 +238,30 @@ def test_migrate(self): executor.migrate() self.assertEqual(migrated, {'1': True, '2': True, '3': True}) - def test_showmigrations(self): + def test_show_migrations(self): connection = mock.Mock() executor = migration_executor.MigrationExecutor( connection, self.TEST_MIGRATIONS_DIR) - first = migration.Migration('1', None) - second = migration.Migration('2', '1') - third = migration.Migration('3', '2') + first = migration.Migration('abcdef', None, '1') + second = migration.Migration('012345', 'abcdef', '2') + third = migration.Migration('6abcde', '012345', '3') with mock.patch.object(executor, 'migrations') as migrations: migrations.return_value = [first, second, third] - migrated = {'1': True, '2': False, '3': False} + migrated = {'abcdef': True, '012345': False, '6abcde': False} with mock.patch.object(executor, '_migration_status_map', migrated): with mock.patch('sys.stdout', new_callable=StringIO) as mock_stdout: executor.show_migrations() - self.assertEqual("[ ] Migration 3\n[ ] Migration 2\n[X] Migration 1\n", mock_stdout.getvalue()) + self.assertEqual("[ ] 6abcde, 3\n[ ] 012345, 2\n[X] abcdef, 1\n", mock_stdout.getvalue()) def test_rollback(self): connection = mock.Mock() executor = migration_executor.MigrationExecutor( connection, self.TEST_MIGRATIONS_DIR) - first = migration.Migration('1', None) - second = migration.Migration('2', '1') - third = migration.Migration('3', '2') + first = migration.Migration('1', None, '1') + second = migration.Migration('2', '1', '2') + third = migration.Migration('3', '2', '3') with mock.patch.object(executor, 'migrations') as migrations: migrations.return_value = [first, second, third] migrated = {'1': True, '2': False, '3': False}