From 49e694708d1d9056895fdaba45fd04f8a2eb03a1 Mon Sep 17 00:00:00 2001
From: Ralph Bean <rbean@redhat.com>
Date: Feb 14 2018 17:12:27 +0000
Subject: [PATCH 1/3] Split migration.


For #121 and #115.

---

diff --git a/waiverdb/migrations/env.py b/waiverdb/migrations/env.py
index 8d12f2e..c75f46f 100644
--- a/waiverdb/migrations/env.py
+++ b/waiverdb/migrations/env.py
@@ -73,6 +73,7 @@ def run_migrations_online():
     context.configure(connection=connection,
                       target_metadata=target_metadata,
                       process_revision_directives=process_revision_directives,
+                      transaction_per_migration=True,
                       **current_app.extensions['migrate'].configure_args)
 
     try:
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
new file mode 100644
index 0000000..2615bfb
--- /dev/null
+++ b/waiverdb/migrations/versions/71b84ccc31bb_migrate_records_from_old_format_to_new.py
@@ -0,0 +1,58 @@
+"""migrate records from old format to new.
+
+Revision ID: 71b84ccc31bb
+Revises: f2772c2c64a6
+Create Date: 2018-02-14 12:04:34.688790
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '71b84ccc31bb'
+down_revision = 'f2772c2c64a6'
+
+import requests
+
+from waiverdb.api_v1 import get_resultsdb_result
+from waiverdb.models import db, Waiver
+
+
+def convert_id_to_subject_and_testcase(result_id):
+    try:
+        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))
+        else:
+            raise RuntimeError('Failed looking up result in Resultsdb: %s' % e)
+    except Exception as e:
+        raise RuntimeError('Failed looking up result in Resultsdb: %s' % e)
+    if 'original_spec_nvr' in result['data']:
+        subject = {'original_spec_nvr': result['data']['original_spec_nvr'][0]}
+    else:
+        if result['data']['type'][0] == 'koji_build' or \
+           result['data']['type'][0] == 'bodhi_update':
+            SUBJECT_KEYS = ['item', 'type']
+            subject = dict([(k, v[0]) for k, v in result['data'].items()
+                            if k in SUBJECT_KEYS])
+        else:
+            raise RuntimeError('Unable to determine subject for result id %s' % (result_id))
+    testcase = result['testcase']['name']
+    return (subject, testcase)
+
+
+def upgrade():
+    # querying resultsdb for the corresponding subject/testcase for each result_id
+    waivers = Waiver.query.all()
+    for waiver in waivers:
+        subject, testcase = convert_id_to_subject_and_testcase(waiver.result_id)
+        waiver.subject = subject
+        waiver.testcase = testcase
+        db.session.commit()
+
+
+def downgrade():
+    # It shouldn't be possible to downgrade this change.
+    # Because the result_id field will not be populated with data anymore.
+    # If the user tries to downgrade "result_id" should be not null once again
+    # like in the old version of the schema, but the value is not longer available
+    raise RuntimeError('Irreversible migration')
diff --git a/waiverdb/migrations/versions/ed43eb9b221c_set_nullable_on_new_and_old_fields.py b/waiverdb/migrations/versions/ed43eb9b221c_set_nullable_on_new_and_old_fields.py
new file mode 100644
index 0000000..1ffe0b1
--- /dev/null
+++ b/waiverdb/migrations/versions/ed43eb9b221c_set_nullable_on_new_and_old_fields.py
@@ -0,0 +1,30 @@
+"""set nullable on new and old fields.
+
+Revision ID: ed43eb9b221c
+Revises: 71b84ccc31bb
+Create Date: 2018-02-14 12:09:42.877375
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'ed43eb9b221c'
+down_revision = '71b84ccc31bb'
+
+from alembic import op
+
+
+def upgrade():
+    # SQLite has some problem in dropping/altering columns.
+    # So in this way Alembic should do some behind the scenes
+    # with: make new table - copy data - drop old table - rename new table
+    with op.batch_alter_table('waiver') as batch_op:
+        batch_op.alter_column('subject', nullable=False)
+        batch_op.alter_column('testcase', nullable=False)
+        batch_op.alter_column('result_id', nullable=True)
+
+
+def downgrade():
+    with op.batch_alter_table('waiver') as batch_op:
+        batch_op.alter_column('subject', nullable=True)
+        batch_op.alter_column('testcase', nullable=True)
+        batch_op.alter_column('result_id', nullable=False)
diff --git a/waiverdb/migrations/versions/f2772c2c64a6_waive_absence_of_result.py b/waiverdb/migrations/versions/f2772c2c64a6_waive_absence_of_result.py
index 8238b1d..0d06394 100644
--- a/waiverdb/migrations/versions/f2772c2c64a6_waive_absence_of_result.py
+++ b/waiverdb/migrations/versions/f2772c2c64a6_waive_absence_of_result.py
@@ -8,10 +8,6 @@ Create Date: 2017-12-04 10:03:54.792758
 
 from alembic import op
 import sqlalchemy as sa
-import requests
-
-from waiverdb.api_v1 import get_resultsdb_result
-from waiverdb.models import db, Waiver
 
 
 # revision identifiers, used by Alembic.
@@ -19,54 +15,11 @@ revision = 'f2772c2c64a6'
 down_revision = '0a74cdab732a'
 
 
-def convert_id_to_subject_and_testcase(result_id):
-    try:
-        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))
-        else:
-            raise RuntimeError('Failed looking up result in Resultsdb: %s' % e)
-    except Exception as e:
-        raise RuntimeError('Failed looking up result in Resultsdb: %s' % e)
-    if 'original_spec_nvr' in result['data']:
-        subject = {'original_spec_nvr': result['data']['original_spec_nvr'][0]}
-    else:
-        if result['data']['type'][0] == 'koji_build' or \
-           result['data']['type'][0] == 'bodhi_update':
-            SUBJECT_KEYS = ['item', 'type']
-            subject = dict([(k, v[0]) for k, v in result['data'].items()
-                            if k in SUBJECT_KEYS])
-        else:
-            raise RuntimeError('Unable to determine subject for result id %s' % (result_id))
-    testcase = result['testcase']['name']
-    return (subject, testcase)
-
-
 def upgrade():
     op.add_column('waiver', sa.Column('subject', sa.Text(), nullable=True, index=True))
     op.add_column('waiver', sa.Column('testcase', sa.Text(), nullable=True, index=True))
 
-    # querying resultsdb for the corresponding subject/testcase for each result_id
-    waivers = Waiver.query.all()
-    for waiver in waivers:
-        subject, testcase = convert_id_to_subject_and_testcase(waiver.result_id)
-        waiver.subject = subject
-        waiver.testcase = testcase
-        db.session.commit()
-
-    # SQLite has some problem in dropping/altering columns.
-    # So in this way Alembic should do some behind the scenes
-    # with: make new table - copy data - drop old table - rename new table
-    with op.batch_alter_table('waiver') as batch_op:
-        batch_op.alter_column('subject', nullable=False)
-        batch_op.alter_column('testcase', nullable=False)
-        batch_op.alter_column('result_id', nullable=True)
-
 
 def downgrade():
-    # It shouldn't be possible to downgrade this change.
-    # Because the result_id field will not be populated with data anymore.
-    # If the user tries to downgrade "result_id" should be not null once again
-    # like in the old version of the schema, but the value is not longer available
-    raise RuntimeError('Irreversible migration')
+    op.drop_column('waiver', 'subject')
+    op.drop_column('waiver', 'testcase')

From ff7bd4982068ad25f3b9bd8cea66ee4c61804eda Mon Sep 17 00:00:00 2001
From: Ralph Bean <rbean@redhat.com>
Date: Feb 14 2018 19:27:07 +0000
Subject: [PATCH 2/3] Some log statements.


---

diff --git a/waiverdb/migrations/env.py b/waiverdb/migrations/env.py
index c75f46f..c51beb2 100644
--- a/waiverdb/migrations/env.py
+++ b/waiverdb/migrations/env.py
@@ -65,6 +65,7 @@ def run_migrations_online():
                 directives[:] = []
                 logger.info('No changes in schema detected.')
 
+    logger.info('Connecting...')
     engine = engine_from_config(config.get_section(config.config_ini_section),
                                 prefix='sqlalchemy.',
                                 poolclass=pool.NullPool)
@@ -77,9 +78,14 @@ def run_migrations_online():
                       **current_app.extensions['migrate'].configure_args)
 
     try:
+        logger.info('Beginning outermost transaction.')
         with context.begin_transaction():
+            logger.info('Starting migrations.')
             context.run_migrations()
+            logger.info('Done with migrations.')
+        logger.info('Outermost transaction released.')
     finally:
+        logger.info('Closing connection.')
         connection.close()
 
 if context.is_offline_mode():

From 89d4374c3dcebbfd7ffbb60b69684f8c833f5da8 Mon Sep 17 00:00:00 2001
From: Ralph Bean <rbean@redhat.com>
Date: Feb 14 2018 20:28:43 +0000
Subject: [PATCH 3/3] Put this operation on a session associated with the alembic op.


---

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 2615bfb..a5753fd 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
@@ -10,10 +10,13 @@ Create Date: 2018-02-14 12:04:34.688790
 revision = '71b84ccc31bb'
 down_revision = 'f2772c2c64a6'
 
+from alembic import op
+import sqlalchemy as sa
+
 import requests
 
 from waiverdb.api_v1 import get_resultsdb_result
-from waiverdb.models import db, Waiver
+from waiverdb.models import Waiver
 
 
 def convert_id_to_subject_and_testcase(result_id):
@@ -41,13 +44,24 @@ def convert_id_to_subject_and_testcase(result_id):
 
 
 def upgrade():
-    # querying resultsdb for the corresponding subject/testcase for each result_id
-    waivers = Waiver.query.all()
-    for waiver in waivers:
-        subject, testcase = convert_id_to_subject_and_testcase(waiver.result_id)
-        waiver.subject = subject
-        waiver.testcase = testcase
-        db.session.commit()
+    # 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()
 
 
 def downgrade():

