Merge lp:~jml/pkgme-devportal/schema-changes into lp:pkgme-devportal

[Jonathan Lange]

Add a schema patch to add 'architecture' column to libdep table.

The patch to add architecture.
Updates our tests to use the schema & patch.

[Michael Vogt (mvo)]

Thanks for this branch.

Is there a benefit of doing this manually over using the storm support for schema updates?
There is some outline how that is done in lp:~mvo/pkgme-devportal/schemas-via-storm

The code itself looks fine and +1.

[Jonathan Lange (jml)]

On 16 August 2012 11:38, Michael Vogt <email address hidden> wrote:
> Thanks for this branch.
>
> Is there a benefit of doing this manually over using the storm support for schema updates?
> There is some outline how that is done in lp:~mvo/pkgme-devportal/schemas-via-storm
>

Good question. I don't know the answer, but here are some thoughts::

 * webops are familiar with applying direct SQL patches
 * less can go wrong with direct SQL patches: missing dependencies,
PYTHONPATH issues, etc.
 * we don't want any schema changing code in the service itself, so
we'd have storm migration code only for tests and for deployment
 * I am personally unfamiliar with storm migrations support, and would
have to learn it to do this particular patch
 * I don't know what benefit storm migration support provides
 * this is our first schema update, and I'd like to reduce variables
 * this is our first schema update, there will be others and we are
free to do those in any fashion we wish

That sounds pretty negative, but that's mostly because I'm ignorant of
the positives.

> The code itself looks fine and +1.

Thanks,
jml

[Jonathan Lange (jml)]

On 16 August 2012 11:38, Michael Vogt <email address hidden> wrote:
> Thanks for this branch.
>
> Is there a benefit of doing this manually over using the storm support for schema updates?
> There is some outline how that is done in lp:~mvo/pkgme-devportal/schemas-via-storm
>

Good question. I don't know the answer, but here are some thoughts::

 * webops are familiar with applying direct SQL patches
 * less can go wrong with direct SQL patches: missing dependencies,
PYTHONPATH issues, etc.
 * we don't want any schema changing code in the service itself, so
we'd have storm migration code only for tests and for deployment
 * I am personally unfamiliar with storm migrations support, and would
have to learn it to do this particular patch
 * I don't know what benefit storm migration support provides
 * this is our first schema update, and I'd like to reduce variables
 * this is our first schema update, there will be others and we are
free to do those in any fashion we wish

That sounds pretty negative, but that's mostly because I'm ignorant of
the positives.

> The code itself looks fine and +1.

Thanks,
jml

[Jonathan Lange (jml)]

Taking mvo's +1 as an approval.

review: Approve

[Michael Vogt (mvo)]

review: Approve

=== added file 'db/patch-00001.sql'
--- db/patch-00001.sql	1970-01-01 00:00:00 +0000
+++ db/patch-00001.sql	2012-08-16 10:25:35 +0000
@@ -0,0 +1,3 @@
+
+ALTER TABLE libdep
+  ADD COLUMN architecture TEXT;

=== modified file 'devportalbinary/testing.py'
--- devportalbinary/testing.py	2012-08-10 15:11:35 +0000
+++ devportalbinary/testing.py	2012-08-16 10:25:35 +0000
@@ -89,9 +89,11 @@
         os.path.abspath(__file__))), 'db', name)
 
 
-def get_postgres_schema_create_sql():
-    with open(get_db_schema_file_path('postgres_schema.sql')) as f:
-        return f.read()
+def get_db_schema_queries(filenames):
+    for filename in filenames:
+        path = get_db_schema_file_path(filename)
+        with open(path) as f:
+            yield f.read()
 
 
 class PostgresDatabaseFixture(Fixture):
@@ -110,10 +112,14 @@
 
     def create_db(self):
         self.cluster.createdb(self.db_name)
-        postgres_schema_create = get_postgres_schema_create_sql()
+        queries = ['postgres_schema.sql', 'patch-00001.sql']
+        for patch in get_db_schema_queries(queries):
+            self._execute(patch)
+
+    def _execute(self, query):
         with closing(self.cluster.connect(self.db_name)) as conn:
             cur = conn.cursor()
-            cur.execute(postgres_schema_create)
+            cur.execute(query)
             conn.commit()
 
     def close_connection(self):