From 316e1dfddeb7955c3bb8a5183c53024c68184a22 Mon Sep 17 00:00:00 2001 From: Elrond Date: Sat, 24 Nov 2012 19:19:18 +0100 Subject: SQL Migrations: Rewrite table creation completely. We have migrations creating new tables. Those currently use "raw" table definitions. This easily gives errors (we already had this problem). So instead rewrite those to use declarative tables and use those to create new tables. Just copy the new table over to the migration, strip it down to the bare minimum, rename to _v0, base it on declarative_base() and be done! Do this for the current migrations. --- mediagoblin/db/sql/migrations.py | 78 +++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 33 deletions(-) (limited to 'mediagoblin/db/sql/migrations.py') diff --git a/mediagoblin/db/sql/migrations.py b/mediagoblin/db/sql/migrations.py index 1d822cd9..bc68caa3 100644 --- a/mediagoblin/db/sql/migrations.py +++ b/mediagoblin/db/sql/migrations.py @@ -17,11 +17,12 @@ import datetime from sqlalchemy import (MetaData, Table, Column, Boolean, SmallInteger, - Integer, Unicode, UnicodeText, DateTime, ForeignKey) + Integer, Unicode, UnicodeText, DateTime, + ForeignKey, UniqueConstraint) +from sqlalchemy.ext.declarative import declarative_base from mediagoblin.db.sql.util import RegisterMigration -from mediagoblin.db.sql.models import MediaEntry, Collection, User, \ - ProcessingMetaData +from mediagoblin.db.sql.models import MediaEntry, Collection, User MIGRATIONS = {} @@ -65,29 +66,40 @@ def add_transcoding_progress(db_conn): db_conn.commit() +class Collection_v0(declarative_base()): + __tablename__ = "core__collections" + + id = Column(Integer, primary_key=True) + title = Column(Unicode, nullable=False) + slug = Column(Unicode) + created = Column(DateTime, nullable=False, default=datetime.datetime.now, + index=True) + description = Column(UnicodeText) + creator = Column(Integer, ForeignKey(User.id), nullable=False) + items = Column(Integer, default=0) + +class CollectionItem_v0(declarative_base()): + __tablename__ = "core__collection_items" + + id = Column(Integer, primary_key=True) + media_entry = Column( + Integer, ForeignKey(MediaEntry.id), nullable=False, index=True) + collection = Column(Integer, ForeignKey(Collection.id), nullable=False) + note = Column(UnicodeText, nullable=True) + added = Column(DateTime, nullable=False, default=datetime.datetime.now) + position = Column(Integer) + + ## This should be activated, normally. + ## But this would change the way the next migration used to work. + ## So it's commented for now. + # __table_args__ = ( + # UniqueConstraint('collection', 'media_entry'), + # {}) + @RegisterMigration(4, MIGRATIONS) def add_collection_tables(db_conn): - metadata = MetaData(bind=db_conn.bind) - - collection = Table('core__collections', metadata, - Column('id', Integer, primary_key=True), - Column('title', Unicode, nullable=False), - Column('slug', Unicode), - Column('created', DateTime, nullable=False, default=datetime.datetime.now, index=True), - Column('description', UnicodeText), - Column('creator', Integer, ForeignKey(User.id), nullable=False), - Column('items', Integer, default=0)) - - collection_item = Table('core__collection_items', metadata, - Column('id', Integer, primary_key=True), - Column('media_entry', Integer, ForeignKey(MediaEntry.id), nullable=False, index=True), - Column('collection', Integer, ForeignKey(Collection.id), nullable=False), - Column('note', UnicodeText, nullable=True), - Column('added', DateTime, nullable=False, default=datetime.datetime.now), - Column('position', Integer)) - - collection.create() - collection_item.create() + Collection_v0.__table__.create(db_conn.bind) + CollectionItem_v0.__table__.create(db_conn.bind) db_conn.commit() @@ -104,15 +116,15 @@ def add_mediaentry_collected(db_conn): db_conn.commit() -@RegisterMigration(6, MIGRATIONS) -def create_processing_metadata_table(db): - metadata = MetaData(bind=db.bind) +class ProcessingMetaData_v0(declarative_base()): + __tablename__ = 'core__processing_metadata' - metadata_table = Table('core__processing_metadata', metadata, - Column('id', Integer, primary_key=True), - Column('media_entry_id', Integer, ForeignKey(MediaEntry.id), - nullable=False, index=True), - Column('callback_url', Unicode)) + id = Column(Integer, primary_key=True) + media_entry_id = Column(Integer, ForeignKey(MediaEntry.id), nullable=False, + index=True) + callback_url = Column(Unicode) - metadata_table.create() +@RegisterMigration(6, MIGRATIONS) +def create_processing_metadata_table(db): + ProcessingMetaData_v0.__table__.create(db.bind) db.commit() -- cgit v1.2.3 From 0f14c362c33408d4642372392fa56353946d5cdc Mon Sep 17 00:00:00 2001 From: Elrond Date: Wed, 28 Nov 2012 16:44:43 +0100 Subject: Fixing our broken CollectionItem unique constraint. This one seems to work nicely in all relevant situations. See comments inside the source. --- mediagoblin/db/sql/migrations.py | 46 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) (limited to 'mediagoblin/db/sql/migrations.py') diff --git a/mediagoblin/db/sql/migrations.py b/mediagoblin/db/sql/migrations.py index bc68caa3..274843ff 100644 --- a/mediagoblin/db/sql/migrations.py +++ b/mediagoblin/db/sql/migrations.py @@ -18,8 +18,10 @@ import datetime from sqlalchemy import (MetaData, Table, Column, Boolean, SmallInteger, Integer, Unicode, UnicodeText, DateTime, - ForeignKey, UniqueConstraint) + ForeignKey) +from sqlalchemy.exc import ProgrammingError from sqlalchemy.ext.declarative import declarative_base +from migrate.changeset.constraint import UniqueConstraint from mediagoblin.db.sql.util import RegisterMigration from mediagoblin.db.sql.models import MediaEntry, Collection, User @@ -92,15 +94,20 @@ class CollectionItem_v0(declarative_base()): ## This should be activated, normally. ## But this would change the way the next migration used to work. ## So it's commented for now. - # __table_args__ = ( - # UniqueConstraint('collection', 'media_entry'), - # {}) + __table_args__ = ( + UniqueConstraint('collection', 'media_entry'), + {}) + +collectionitem_unique_constraint_done = False @RegisterMigration(4, MIGRATIONS) def add_collection_tables(db_conn): Collection_v0.__table__.create(db_conn.bind) CollectionItem_v0.__table__.create(db_conn.bind) + global collectionitem_unique_constraint_done + collectionitem_unique_constraint_done = True + db_conn.commit() @@ -128,3 +135,34 @@ class ProcessingMetaData_v0(declarative_base()): def create_processing_metadata_table(db): ProcessingMetaData_v0.__table__.create(db.bind) db.commit() + +@RegisterMigration(7, MIGRATIONS) +def fix_CollectionItem_v0_constraint(db_conn): + """Add the forgotten Constraint on CollectionItem""" + + global collectionitem_unique_constraint_done + if collectionitem_unique_constraint_done: + print "Okay, already done, short circuit" + # Reset it. Maybe the whole thing gets run again + # For a different db? + collectionitem_unique_constraint_done = False + return + + metadata = MetaData(bind=db_conn.bind) + + CollectionItem_table = Table('core__collection_items', + metadata, autoload=True, autoload_with=db_conn.bind) + + constraint = UniqueConstraint('collection', 'media_entry', + name='core__collection_items_collection_media_entry_key', + table=CollectionItem_table) + + try: + constraint.create() + except ProgrammingError as exc: + print "***", repr(exc) + print "***", repr(exc.statement) + print "***", repr(exc.params) + print "***", repr(exc.orig) + print "*** Ignoring it, you probably have a fresh install from a recent git." + db_conn.commit() -- cgit v1.2.3 From ea91c183ff8c6c877d3403f354c92b18a3788860 Mon Sep 17 00:00:00 2001 From: Elrond Date: Sun, 9 Dec 2012 19:50:33 +0100 Subject: UniqueConstraing migration: Adding the explaining comments. Add a lengthy comment explaining all the variants. --- mediagoblin/db/sql/migrations.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'mediagoblin/db/sql/migrations.py') diff --git a/mediagoblin/db/sql/migrations.py b/mediagoblin/db/sql/migrations.py index 274843ff..66557912 100644 --- a/mediagoblin/db/sql/migrations.py +++ b/mediagoblin/db/sql/migrations.py @@ -136,6 +136,29 @@ def create_processing_metadata_table(db): ProcessingMetaData_v0.__table__.create(db.bind) db.commit() + +# Okay, problem being: +# Migration #4 forgot to add the uniqueconstraint for the +# new tables. While creating the tables from scratch had +# the constraint enabled. +# +# So we have three situations that should end up at the same +# db layout: +# +# 1. Fresh install. +# Well, easy. Just uses the tables in models.py +# 2. Fresh install using a git version just before this migration +# The tables are all there, the unique constraint is also there. +# This migration should do nothing. +# But as we can't detect the uniqueconstraint easily, +# this migration just adds the constraint again. +# And possibly fails very loud. But ignores the failure. +# 3. old install, not using git, just releases. +# This one will get the new tables in #4 (now with constraint!) +# And this migration is just skipped silently. +# 4. old install, always on latest git. +# This one has the tables, but lacks the constraint. +# So this mitration adds the constraint. @RegisterMigration(7, MIGRATIONS) def fix_CollectionItem_v0_constraint(db_conn): """Add the forgotten Constraint on CollectionItem""" -- cgit v1.2.3 From a64abbb10abf025d10dbc1a49979c7c8f4d74895 Mon Sep 17 00:00:00 2001 From: Elrond Date: Sun, 9 Dec 2012 20:14:15 +0100 Subject: Fix typo, disable debugging. --- mediagoblin/db/sql/migrations.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'mediagoblin/db/sql/migrations.py') diff --git a/mediagoblin/db/sql/migrations.py b/mediagoblin/db/sql/migrations.py index 66557912..b4c91eee 100644 --- a/mediagoblin/db/sql/migrations.py +++ b/mediagoblin/db/sql/migrations.py @@ -142,7 +142,7 @@ def create_processing_metadata_table(db): # new tables. While creating the tables from scratch had # the constraint enabled. # -# So we have three situations that should end up at the same +# So we have four situations that should end up at the same # db layout: # # 1. Fresh install. @@ -158,14 +158,13 @@ def create_processing_metadata_table(db): # And this migration is just skipped silently. # 4. old install, always on latest git. # This one has the tables, but lacks the constraint. -# So this mitration adds the constraint. +# So this migration adds the constraint. @RegisterMigration(7, MIGRATIONS) def fix_CollectionItem_v0_constraint(db_conn): """Add the forgotten Constraint on CollectionItem""" global collectionitem_unique_constraint_done if collectionitem_unique_constraint_done: - print "Okay, already done, short circuit" # Reset it. Maybe the whole thing gets run again # For a different db? collectionitem_unique_constraint_done = False -- cgit v1.2.3 From 78fd5581a9fec142a1236eb90508724281f7f5ba Mon Sep 17 00:00:00 2001 From: Christopher Allan Webber Date: Mon, 10 Dec 2012 16:14:28 -0600 Subject: Remove print statments from this migration. The reason for wanting to give extra information to the user (this is a very special case migration) is good, but we don't have a nice "official" way to capture and present that information during tests, so removing this. --- mediagoblin/db/sql/migrations.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) (limited to 'mediagoblin/db/sql/migrations.py') diff --git a/mediagoblin/db/sql/migrations.py b/mediagoblin/db/sql/migrations.py index b4c91eee..99448bfa 100644 --- a/mediagoblin/db/sql/migrations.py +++ b/mediagoblin/db/sql/migrations.py @@ -181,10 +181,9 @@ def fix_CollectionItem_v0_constraint(db_conn): try: constraint.create() - except ProgrammingError as exc: - print "***", repr(exc) - print "***", repr(exc.statement) - print "***", repr(exc.params) - print "***", repr(exc.orig) - print "*** Ignoring it, you probably have a fresh install from a recent git." + except ProgrammingError: + # User probably has an install that was run since the + # collection tables were added, so we don't need to run this migration. + pass + db_conn.commit() -- cgit v1.2.3