Merge lp:~bloodearnest/lazr-postgresql/schema into lp:lazr-postgresql
- schema
- Merge into trunk
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 |
Related bugs: |
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 : | # |
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: |
Looks OK - just the SQL injection issue jumps out (see inline)