From 31d5a343cee7577dd699119f1d4ac7ffbd426272 Mon Sep 17 00:00:00 2001
From: Dan Callaghan <dcallagh@redhat.com>
Date: Mar 28 2018 05:05:51 +0000
Subject: avoid using model classes in Alembic migrations


... otherwise we can never change or remove the columns used in the
migrations. The migrations need to use raw SQL and so-called
"lightweight" table definitions for interacting with the database
instead, according to the state of the schema at the point in time the
migration was written for.

This fixes migration 71b84ccc31bb (result_id -> subject, testcase)
allowing us to drop the result_id column from the model definition. It
will also allow us to write further migrations in future to change/drop
the subject column.

---

diff --git a/waiverdb/migrations/versions/71b84ccc31bb_migrate_records_from_old_format_to_new.py b/waiverdb/migrations/versions/71b84ccc31bb_migrate_records_from_old_format_to_new.py
index a5753fd..c34a427 100644
--- a/waiverdb/migrations/versions/71b84ccc31bb_migrate_records_from_old_format_to_new.py
+++ b/waiverdb/migrations/versions/71b84ccc31bb_migrate_records_from_old_format_to_new.py
@@ -11,12 +11,14 @@ revision = '71b84ccc31bb'
 down_revision = 'f2772c2c64a6'
 
 from alembic import op
-import sqlalchemy as sa
+# These are the "lightweight" SQL expression versions (not using metadata):
+from sqlalchemy.sql.expression import table, column, select, update
+from sqlalchemy.sql.sqltypes import Integer, Text
 
+import json
 import requests
 
 from waiverdb.api_v1 import get_resultsdb_result
-from waiverdb.models import Waiver
 
 
 def convert_id_to_subject_and_testcase(result_id):
@@ -24,11 +26,11 @@ def convert_id_to_subject_and_testcase(result_id):
         result = get_resultsdb_result(result_id)
     except requests.HTTPError as e:
         if e.response.status_code == 404:
-            raise RuntimeError('Result id %s not found in Resultsdb' % (result_id))
+            raise RuntimeError('Result id %s not found in Resultsdb' % (result_id)) from e
         else:
-            raise RuntimeError('Failed looking up result in Resultsdb: %s' % e)
+            raise RuntimeError('Failed looking up result in Resultsdb') from e
     except Exception as e:
-        raise RuntimeError('Failed looking up result in Resultsdb: %s' % e)
+        raise RuntimeError('Failed looking up result in Resultsdb') from e
     if 'original_spec_nvr' in result['data']:
         subject = {'original_spec_nvr': result['data']['original_spec_nvr'][0]}
     else:
@@ -44,24 +46,20 @@ def convert_id_to_subject_and_testcase(result_id):
 
 
 def upgrade():
+    # Lightweight table definition for producing UPDATE queries.
+    waiver_table = table('waiver',
+                         column('id', type_=Integer),
+                         column('result_id', type_=Integer),
+                         column('subject', type_=Text),
+                         column('testcase', type_=Text))
     # Get a session asociated with the alembic upgrade operation.
     connection = op.get_bind()
-    Session = sa.orm.sessionmaker()
-    session = Session(bind=connection)
-
-    try:
-        # querying resultsdb for the corresponding subject/testcase.
-        waivers = session.query(Waiver).all()
-        for waiver in waivers:
-            subject, testcase = convert_id_to_subject_and_testcase(waiver.result_id)
-            waiver.subject = subject
-            waiver.testcase = testcase
-        session.commit()
-    except:
-        session.rollback()
-        raise
-    finally:
-        session.close()
+    rows = connection.execute(select([waiver_table.c.id, waiver_table.c.result_id]))
+    for waiver_id, result_id in rows:
+        subject, testcase = convert_id_to_subject_and_testcase(result_id)
+        connection.execute(update(waiver_table)
+                           .where(waiver_table.c.id == waiver_id)
+                           .values(subject=json.dumps(subject), testcase=testcase))
 
 
 def downgrade():
diff --git a/waiverdb/models/waivers.py b/waiverdb/models/waivers.py
index e6f2a83..346af32 100644
--- a/waiverdb/models/waivers.py
+++ b/waiverdb/models/waivers.py
@@ -7,7 +7,6 @@ from sqlalchemy import or_, and_, cast
 
 class Waiver(db.Model):
     id = db.Column(db.Integer, primary_key=True)
-    result_id = db.Column(db.Integer, nullable=True)
     subject = db.Column(EqualityComparableJSONType, nullable=False)
     testcase = db.Column(db.Text, nullable=False, index=True)
     username = db.Column(db.String(255), nullable=False)
@@ -31,9 +30,9 @@ class Waiver(db.Model):
         self.proxied_by = proxied_by
 
     def __repr__(self):
-        return '%s(result_id=%r, subject=%r, testcase=%r, username=%r, product_version=%r,\
-                waived=%r)' % (self.__class__.__name__, self.result_id, self.subject, self.testcase,
-                               self.username, self.product_version, self.waived)
+        return '%s(subject=%r, testcase=%r, username=%r, product_version=%r, waived=%r)' % (
+                self.__class__.__name__, self.subject, self.testcase, self.username,
+                self.product_version, self.waived)
 
     @classmethod
     def by_results(cls, query, results):