launchpad-reviewers team mailing list archive

[Simon Davy <simon.davy@xxxxxxxxxxxxx>]

Simon Davy has proposed merging lp:~bloodearnest/lazr-postgresql/schema into lp:lazr-postgresql.

Commit message:
Add support for specifying a schema to apply migrations to

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~bloodearnest/lazr-postgresql/schema/+merge/328173

Add support for explicit schemas
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~bloodearnest/lazr-postgresql/schema into lp:lazr-postgresql.

=== modified file 'setup.py'
--- setup.py	2016-06-24 01:24:27 +0000
+++ setup.py	2017-07-27 15:27:26 +0000
@@ -29,7 +29,7 @@
     os.path.dirname(__file__), 'README'), 'rb').read().decode('UTF-8')
 
 setup(name="lazr.postgresql",
-      version="0.0.2",
+      version="0.0.3",
       description=\
               "LAZR postgresql specific support code.",
       long_description=description,

=== modified file 'src/lazr/postgresql/migrate.py'
--- src/lazr/postgresql/migrate.py	2016-08-03 02:12:30 +0000
+++ src/lazr/postgresql/migrate.py	2017-07-27 15:27:26 +0000
@@ -80,6 +80,8 @@
         action="store_true")
     parser.add_option('--verbose', '-v', help="Show more output",
         action="store_true")
+    parser.add_option('--schema', help="Which postgres schema to use",
+        default="public")
     options, args = parser.parse_args(argv[1:])
     level = logging.INFO
     if options.verbose:
@@ -87,6 +89,13 @@
     logging.basicConfig(level=level)
     if len(args) != 2:
         parser.error("Unexpected number of arguments. got %r" % (args,))
+
+    def connect():
+        conn = psycopg2.connect(args[0])
+        conn.cursor().execute('SET search_path TO ' + options.schema)
+        return conn
+
     return upgrade.upgrade(
-        partial(psycopg2.connect, args[0]), args[1], options.dry_run,
-        options.patch_type, options.apply_all, options.one_only)
+        connect, args[1], options.dry_run,
+        options.patch_type, options.apply_all, options.one_only,
+        options.schema)

=== modified file 'src/lazr/postgresql/tests/test_migrate.py'
--- src/lazr/postgresql/tests/test_migrate.py	2016-08-03 02:12:30 +0000
+++ src/lazr/postgresql/tests/test_migrate.py	2017-07-27 15:27:26 +0000
@@ -33,12 +33,25 @@
     def test_smoketest(self):
         argv = ['lp-migrate', 'connstring', 'patch-dir']
         calls = []
+
+        class Cursor:
+            def execute(self, *args):
+                calls.append(args)
+
+        class Conn:
+            def cursor(self):
+                return Cursor()
+
+        conn = Conn()
+
         def connect(params):
             calls.append(params)
-            return 'connection'
+            return conn
+
         self.useFixture(MonkeyPatch('psycopg2.connect', connect))
+
         def upgrade(connect, schema_dir, dry_run=False, patch_types=None,
-                    apply_all=False, one_only=False):
+                    apply_all=False, one_only=False, schema="public"):
             con = connect()
             calls.append((
                 con, schema_dir, dry_run, patch_types, apply_all, one_only))
@@ -48,5 +61,6 @@
         self.assertEqual(0, main(argv))
         self.assertEqual([
             'connstring',
-            ('connection', 'patch-dir', None, None, None, None),
+            ('SET search_path TO public',),
+            (conn, 'patch-dir', None, None, None, None),
             ], calls)

=== modified file 'src/lazr/postgresql/tests/test_upgrade.py'
--- src/lazr/postgresql/tests/test_upgrade.py	2017-04-23 23:44:51 +0000
+++ src/lazr/postgresql/tests/test_upgrade.py	2017-07-27 15:27:26 +0000
@@ -120,7 +120,7 @@
         expected_calls = [
             'connect',
             ('apply_patches_normal', env, env.patchdir,
-                set([PATCH_STANDARD, PATCH_DIRECT]), True),
+                set([PATCH_STANDARD, PATCH_DIRECT]), True, "public"),
             ('report_patch_times', env, 'patches'),
             'close',
             ]
@@ -134,7 +134,7 @@
         expected_calls = [
             'connect',
             ('apply_patches_normal', env, env.patchdir,
-                set([PATCH_STANDARD, PATCH_DIRECT]), False),
+                set([PATCH_STANDARD, PATCH_DIRECT]), False, "public"),
             ('report_patch_times', env, 'patches'),
             'close',
             ]
@@ -146,7 +146,7 @@
         expected_calls = [
             'connect',
             ('apply_patches_normal', env, env.patchdir,
-                set([PATCH_STANDARD, PATCH_DIRECT]), False),
+                set([PATCH_STANDARD, PATCH_DIRECT]), False, "public"),
             ('report_patch_times', env, 'patches'),
             'close',
             ]
@@ -159,7 +159,7 @@
         expected_calls = [
             'connect',
             ('apply_patches_normal', env, env.patchdir, set([PATCH_STANDARD]),
-                False),
+                False, "public"),
             ('report_patch_times', env, 'patches'),
             'close',
             ]
@@ -170,7 +170,7 @@
         upgrade(env.connect, env.patchdir, one_only=True)
         expected_calls = [
             'connect',
-            ('apply_patches_normal', env, env.patchdir, None, True),
+            ('apply_patches_normal', env, env.patchdir, None, True, "public"),
             'commit',
             ('report_patch_times', env, 'patches'),
             'close',
@@ -182,7 +182,7 @@
         upgrade(env.connect, env.patchdir)
         expected_calls = [
             'connect',
-            ('apply_patches_normal', env, env.patchdir, None, False),
+            ('apply_patches_normal', env, env.patchdir, None, False, "public"),
             'commit',
             ('report_patch_times', env, 'patches'),
             'close',
@@ -195,7 +195,7 @@
         expected_calls = [
             'connect',
             ('apply_patches_normal', env, env.patchdir, [PATCH_STANDARD],
-                False),
+                False, "public"),
             'commit',
             ('report_patch_times', env, 'patches'),
             'close',
@@ -207,13 +207,13 @@
         upgrade(env.connect, env.patchdir, apply_all=True)
         expected_calls = [
             'connect',
-            ('apply_patches_normal', env, env.patchdir, None, False),
-            'commit',
-            ('report_patch_times', env, 'patches'),
-            ('apply_patches_normal', env, env.patchdir, None, False),
-            'commit',
-            ('report_patch_times', env, 'patches'),
-            ('apply_patches_normal', env, env.patchdir, None, False),
+            ('apply_patches_normal', env, env.patchdir, None, False, "public"),
+            'commit',
+            ('report_patch_times', env, 'patches'),
+            ('apply_patches_normal', env, env.patchdir, None, False, "public"),
+            'commit',
+            ('report_patch_times', env, 'patches'),
+            ('apply_patches_normal', env, env.patchdir, None, False, "public"),
             'commit',
             ('report_patch_times', env, None),
             'close',
@@ -225,7 +225,7 @@
         upgrade(env.connect, env.patchdir, apply_all=True, one_only=True)
         expected_calls = [
             'connect',
-            ('apply_patches_normal', env, env.patchdir, None, True),
+            ('apply_patches_normal', env, env.patchdir, None, True, "public"),
             'commit',
             ('report_patch_times', env, 'patches'),
             'close',
@@ -340,7 +340,7 @@
         con = psycopg2.connect(host=self.db.host, database=self.db.database)
         self.addCleanup(con.close)
         calls = []
-        def missing_patches(con, patchdir, patch_types, one_only):
+        def missing_patches(con, patchdir, patch_types, one_only, schema):
             calls.append(patch_types)
             return []
         self.useFixture(MonkeyPatch(
@@ -354,7 +354,7 @@
         con = psycopg2.connect(host=self.db.host, database=self.db.database)
         self.addCleanup(con.close)
         calls = []
-        def missing_patches(con, patchdir, patch_types, one_only):
+        def missing_patches(con, patchdir, patch_types, one_only, schema):
             calls.append(one_only)
             return []
         self.useFixture(MonkeyPatch(

=== modified file 'src/lazr/postgresql/upgrade.py'
--- src/lazr/postgresql/upgrade.py	2017-04-23 23:44:51 +0000
+++ src/lazr/postgresql/upgrade.py	2017-07-27 15:27:26 +0000
@@ -81,7 +81,7 @@
     }
 
 def upgrade(connect, schema_dir, dry_run=False, patch_types=None,
-            apply_all=False, one_only=False):
+            apply_all=False, one_only=False, schema="public"):
     """Main programmatic entrypoint to the migration facility.
 
     :param connect: A callable which returns a postgresql DB connection.
@@ -116,11 +116,11 @@
             if dry_run:
                 log.info("Applying patches in dry-run mode.")
                 patches = apply_patches_normal(
-                    con, schema_dir, patch_types, one_only)
+                    con, schema_dir, patch_types, one_only, schema)
             else:
                 log.info("Applying patches.")
                 patches = apply_patches_normal(
-                    con, schema_dir, patch_types, one_only)
+                    con, schema_dir, patch_types, one_only, schema)
                 con.commit()
             report_patch_times(con, patches)
             if one_only or not apply_all or not patches:
@@ -135,16 +135,16 @@
     """Cannot apply some patch."""
 
 
-def _table_exists(con, tablename):
+def _table_exists(con, tablename, schema):
     """Return True if tablename exists."""
     cur = con.cursor()
     cur.execute("""
         SELECT EXISTS (
             SELECT TRUE FROM information_schema.tables
             WHERE
-                table_schema='public'
+                table_schema='%s'
                 AND table_name='%s')
-            """ % tablename)
+            """ % (schema, tablename))
     return cur.fetchone()[0]
 
 
@@ -222,11 +222,13 @@
     return branch_info
 
 
-def apply_patches_normal(con, patches_dir, patch_types=None, one_only=False):
+def apply_patches_normal(
+        con, patches_dir, patch_types=None, one_only=False, schema="public"):
     """Apply patches to a DB in a transaction on one node."""
     log = logging.getLogger("lazr.postgresql.upgrade")
     patches = missing_patches(
-        con, patches_dir, patch_types=patch_types, one_only=one_only)
+        con, patches_dir, patch_types=patch_types, one_only=one_only,
+        schema=schema)
     if not patches:
         return patches
 
@@ -249,7 +251,8 @@
     return patches
 
 
-def missing_patches(con, patchdir, patch_types=None, one_only=False):
+def missing_patches(
+        con, patchdir, patch_types=None, one_only=False, schema="public"):
     """Calculate the patches that need to be applied for the DB in con.
 
     :param con: A psycopg2 connection.
@@ -260,7 +263,7 @@
     :param one_only: Return at most one patch.
     """
     found_patches = []
-    if not _table_exists(con, SCHEMA_TABLE):
+    if not _table_exists(con, SCHEMA_TABLE, schema):
         found_patches.append(((SCHEMA_MAJOR, 0, 0), None, PATCH_STANDARD))
         existing_patches = set()
     else: