Merge lp:~bloodearnest/lazr-postgresql/schema into lp:lazr-postgresql

Proposed by Simon Davy
Status: Merged
Merged at revision: 25
Proposed branch: lp:~bloodearnest/lazr-postgresql/schema
Merge into: lp:lazr-postgresql
Diff against target: 268 lines (+61/-33)
4 files modified
src/lazr/postgresql/migrate.py (+11/-2)
src/lazr/postgresql/tests/test_migrate.py (+17/-3)
src/lazr/postgresql/tests/test_upgrade.py (+17/-17)
src/lazr/postgresql/upgrade.py (+16/-11)
To merge this branch: bzr merge lp:~bloodearnest/lazr-postgresql/schema
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+328173@code.launchpad.net

Commit message

Add support for specifying a schema to apply migrations to

Description of the change

Add support for explicit schemas

To post a comment you must log in.
Revision history for this message
Adam Collard (adam-collard) wrote :

Looks OK - just the SQL injection issue jumps out (see inline)

Revision history for this message
William Grant (wgrant) wrote :

Your whoami needs fixing and recommitting. And one minor SQL injection comment.

review: Approve (code)
Revision history for this message
Simon Davy (bloodearnest) wrote :

I've fixed the b0rked commit history, and used query params

Revision history for this message
Simon Davy (bloodearnest) wrote :

Looks like I can't top approve.

25. By Simon Davy

Add support for running migrations within a specific schema

Revision history for this message
Simon Davy (bloodearnest) wrote :

Really fixed the history now

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/lazr/postgresql/migrate.py'
2--- src/lazr/postgresql/migrate.py 2016-08-03 02:12:30 +0000
3+++ src/lazr/postgresql/migrate.py 2017-08-15 14:00:53 +0000
4@@ -80,6 +80,8 @@
5 action="store_true")
6 parser.add_option('--verbose', '-v', help="Show more output",
7 action="store_true")
8+ parser.add_option('--schema', help="Which postgres schema to use",
9+ default="public")
10 options, args = parser.parse_args(argv[1:])
11 level = logging.INFO
12 if options.verbose:
13@@ -87,6 +89,13 @@
14 logging.basicConfig(level=level)
15 if len(args) != 2:
16 parser.error("Unexpected number of arguments. got %r" % (args,))
17+
18+ def connect():
19+ conn = psycopg2.connect(args[0])
20+ conn.cursor().execute('SET search_path TO %s', [options.schema])
21+ return conn
22+
23 return upgrade.upgrade(
24- partial(psycopg2.connect, args[0]), args[1], options.dry_run,
25- options.patch_type, options.apply_all, options.one_only)
26+ connect, args[1], options.dry_run,
27+ options.patch_type, options.apply_all, options.one_only,
28+ options.schema)
29
30=== modified file 'src/lazr/postgresql/tests/test_migrate.py'
31--- src/lazr/postgresql/tests/test_migrate.py 2016-08-03 02:12:30 +0000
32+++ src/lazr/postgresql/tests/test_migrate.py 2017-08-15 14:00:53 +0000
33@@ -33,12 +33,25 @@
34 def test_smoketest(self):
35 argv = ['lp-migrate', 'connstring', 'patch-dir']
36 calls = []
37+
38+ class Cursor:
39+ def execute(self, *args):
40+ calls.append(args)
41+
42+ class Conn:
43+ def cursor(self):
44+ return Cursor()
45+
46+ conn = Conn()
47+
48 def connect(params):
49 calls.append(params)
50- return 'connection'
51+ return conn
52+
53 self.useFixture(MonkeyPatch('psycopg2.connect', connect))
54+
55 def upgrade(connect, schema_dir, dry_run=False, patch_types=None,
56- apply_all=False, one_only=False):
57+ apply_all=False, one_only=False, schema="public"):
58 con = connect()
59 calls.append((
60 con, schema_dir, dry_run, patch_types, apply_all, one_only))
61@@ -48,5 +61,6 @@
62 self.assertEqual(0, main(argv))
63 self.assertEqual([
64 'connstring',
65- ('connection', 'patch-dir', None, None, None, None),
66+ ('SET search_path TO %s', ['public']),
67+ (conn, 'patch-dir', None, None, None, None),
68 ], calls)
69
70=== modified file 'src/lazr/postgresql/tests/test_upgrade.py'
71--- src/lazr/postgresql/tests/test_upgrade.py 2017-04-23 23:44:51 +0000
72+++ src/lazr/postgresql/tests/test_upgrade.py 2017-08-15 14:00:53 +0000
73@@ -120,7 +120,7 @@
74 expected_calls = [
75 'connect',
76 ('apply_patches_normal', env, env.patchdir,
77- set([PATCH_STANDARD, PATCH_DIRECT]), True),
78+ set([PATCH_STANDARD, PATCH_DIRECT]), True, "public"),
79 ('report_patch_times', env, 'patches'),
80 'close',
81 ]
82@@ -134,7 +134,7 @@
83 expected_calls = [
84 'connect',
85 ('apply_patches_normal', env, env.patchdir,
86- set([PATCH_STANDARD, PATCH_DIRECT]), False),
87+ set([PATCH_STANDARD, PATCH_DIRECT]), False, "public"),
88 ('report_patch_times', env, 'patches'),
89 'close',
90 ]
91@@ -146,7 +146,7 @@
92 expected_calls = [
93 'connect',
94 ('apply_patches_normal', env, env.patchdir,
95- set([PATCH_STANDARD, PATCH_DIRECT]), False),
96+ set([PATCH_STANDARD, PATCH_DIRECT]), False, "public"),
97 ('report_patch_times', env, 'patches'),
98 'close',
99 ]
100@@ -159,7 +159,7 @@
101 expected_calls = [
102 'connect',
103 ('apply_patches_normal', env, env.patchdir, set([PATCH_STANDARD]),
104- False),
105+ False, "public"),
106 ('report_patch_times', env, 'patches'),
107 'close',
108 ]
109@@ -170,7 +170,7 @@
110 upgrade(env.connect, env.patchdir, one_only=True)
111 expected_calls = [
112 'connect',
113- ('apply_patches_normal', env, env.patchdir, None, True),
114+ ('apply_patches_normal', env, env.patchdir, None, True, "public"),
115 'commit',
116 ('report_patch_times', env, 'patches'),
117 'close',
118@@ -182,7 +182,7 @@
119 upgrade(env.connect, env.patchdir)
120 expected_calls = [
121 'connect',
122- ('apply_patches_normal', env, env.patchdir, None, False),
123+ ('apply_patches_normal', env, env.patchdir, None, False, "public"),
124 'commit',
125 ('report_patch_times', env, 'patches'),
126 'close',
127@@ -195,7 +195,7 @@
128 expected_calls = [
129 'connect',
130 ('apply_patches_normal', env, env.patchdir, [PATCH_STANDARD],
131- False),
132+ False, "public"),
133 'commit',
134 ('report_patch_times', env, 'patches'),
135 'close',
136@@ -207,13 +207,13 @@
137 upgrade(env.connect, env.patchdir, apply_all=True)
138 expected_calls = [
139 'connect',
140- ('apply_patches_normal', env, env.patchdir, None, False),
141- 'commit',
142- ('report_patch_times', env, 'patches'),
143- ('apply_patches_normal', env, env.patchdir, None, False),
144- 'commit',
145- ('report_patch_times', env, 'patches'),
146- ('apply_patches_normal', env, env.patchdir, None, False),
147+ ('apply_patches_normal', env, env.patchdir, None, False, "public"),
148+ 'commit',
149+ ('report_patch_times', env, 'patches'),
150+ ('apply_patches_normal', env, env.patchdir, None, False, "public"),
151+ 'commit',
152+ ('report_patch_times', env, 'patches'),
153+ ('apply_patches_normal', env, env.patchdir, None, False, "public"),
154 'commit',
155 ('report_patch_times', env, None),
156 'close',
157@@ -225,7 +225,7 @@
158 upgrade(env.connect, env.patchdir, apply_all=True, one_only=True)
159 expected_calls = [
160 'connect',
161- ('apply_patches_normal', env, env.patchdir, None, True),
162+ ('apply_patches_normal', env, env.patchdir, None, True, "public"),
163 'commit',
164 ('report_patch_times', env, 'patches'),
165 'close',
166@@ -340,7 +340,7 @@
167 con = psycopg2.connect(host=self.db.host, database=self.db.database)
168 self.addCleanup(con.close)
169 calls = []
170- def missing_patches(con, patchdir, patch_types, one_only):
171+ def missing_patches(con, patchdir, patch_types, one_only, schema):
172 calls.append(patch_types)
173 return []
174 self.useFixture(MonkeyPatch(
175@@ -354,7 +354,7 @@
176 con = psycopg2.connect(host=self.db.host, database=self.db.database)
177 self.addCleanup(con.close)
178 calls = []
179- def missing_patches(con, patchdir, patch_types, one_only):
180+ def missing_patches(con, patchdir, patch_types, one_only, schema):
181 calls.append(one_only)
182 return []
183 self.useFixture(MonkeyPatch(
184
185=== modified file 'src/lazr/postgresql/upgrade.py'
186--- src/lazr/postgresql/upgrade.py 2017-04-23 23:44:51 +0000
187+++ src/lazr/postgresql/upgrade.py 2017-08-15 14:00:53 +0000
188@@ -81,7 +81,7 @@
189 }
190
191 def upgrade(connect, schema_dir, dry_run=False, patch_types=None,
192- apply_all=False, one_only=False):
193+ apply_all=False, one_only=False, schema="public"):
194 """Main programmatic entrypoint to the migration facility.
195
196 :param connect: A callable which returns a postgresql DB connection.
197@@ -116,11 +116,11 @@
198 if dry_run:
199 log.info("Applying patches in dry-run mode.")
200 patches = apply_patches_normal(
201- con, schema_dir, patch_types, one_only)
202+ con, schema_dir, patch_types, one_only, schema)
203 else:
204 log.info("Applying patches.")
205 patches = apply_patches_normal(
206- con, schema_dir, patch_types, one_only)
207+ con, schema_dir, patch_types, one_only, schema)
208 con.commit()
209 report_patch_times(con, patches)
210 if one_only or not apply_all or not patches:
211@@ -135,16 +135,18 @@
212 """Cannot apply some patch."""
213
214
215-def _table_exists(con, tablename):
216+def _table_exists(con, tablename, schema):
217 """Return True if tablename exists."""
218 cur = con.cursor()
219 cur.execute("""
220 SELECT EXISTS (
221 SELECT TRUE FROM information_schema.tables
222 WHERE
223- table_schema='public'
224- AND table_name='%s')
225- """ % tablename)
226+ table_schema=%s
227+ AND table_name=%s)
228+ """,
229+ [schema, tablename]
230+ )
231 return cur.fetchone()[0]
232
233
234@@ -222,11 +224,13 @@
235 return branch_info
236
237
238-def apply_patches_normal(con, patches_dir, patch_types=None, one_only=False):
239+def apply_patches_normal(
240+ con, patches_dir, patch_types=None, one_only=False, schema="public"):
241 """Apply patches to a DB in a transaction on one node."""
242 log = logging.getLogger("lazr.postgresql.upgrade")
243 patches = missing_patches(
244- con, patches_dir, patch_types=patch_types, one_only=one_only)
245+ con, patches_dir, patch_types=patch_types, one_only=one_only,
246+ schema=schema)
247 if not patches:
248 return patches
249
250@@ -249,7 +253,8 @@
251 return patches
252
253
254-def missing_patches(con, patchdir, patch_types=None, one_only=False):
255+def missing_patches(
256+ con, patchdir, patch_types=None, one_only=False, schema="public"):
257 """Calculate the patches that need to be applied for the DB in con.
258
259 :param con: A psycopg2 connection.
260@@ -260,7 +265,7 @@
261 :param one_only: Return at most one patch.
262 """
263 found_patches = []
264- if not _table_exists(con, SCHEMA_TABLE):
265+ if not _table_exists(con, SCHEMA_TABLE, schema):
266 found_patches.append(((SCHEMA_MAJOR, 0, 0), None, PATCH_STANDARD))
267 existing_patches = set()
268 else:

Subscribers

People subscribed via source and target branches

to all changes: