Bring DB in sync with the models.py (#1172)

* Bring DB in sync with the models.py

* Make sure that migrations run on multiple dbs

caravel/migrations/versions/3b626e2a6783_sync_db_with_models.py

@@ -0,0 +1,111 @@
+"""Sync DB with the models.py.
+
+Sqlite doesn't support alter on tables, that's why most of the operations
+are surrounded with try except.
+
+Revision ID: 3b626e2a6783
+Revises: 5e4a03ef0bf0
+Create Date: 2016-09-22 10:21:33.618976
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '3b626e2a6783'
+down_revision = 'eca4694defa7'
+
+from alembic import op
+from caravel import db
+from caravel.utils import generic_find_constraint_name, table_has_constraint
+import logging
+import sqlalchemy as sa
+from sqlalchemy.dialects import mysql
+
+
+def upgrade():
+    # cleanup after: https://github.com/airbnb/caravel/pull/1078
+    try:
+        slices_ibfk_1 = generic_find_constraint_name(
+            table='slices', columns={'druid_datasource_id'},
+            referenced='datasources', db=db) or 'slices_ibfk_1'
+        slices_ibfk_2 = generic_find_constraint_name(
+            table='slices', columns={'table_id'},
+            referenced='tables', db=db) or 'slices_ibfk_2'
+
+        with op.batch_alter_table("slices") as batch_op:
+            batch_op.drop_constraint(slices_ibfk_1, type_="foreignkey")
+            batch_op.drop_constraint(slices_ibfk_2, type_="foreignkey")
+            batch_op.drop_column('druid_datasource_id')
+            batch_op.drop_column('table_id')
+    except Exception as e:
+        logging.warning(str(e))
+
+    # fixed issue: https://github.com/airbnb/caravel/issues/466
+    try:
+        with op.batch_alter_table("columns") as batch_op:
+            batch_op.create_foreign_key(
+                None, 'datasources', ['datasource_name'], ['datasource_name'])
+    except Exception as e:
+        logging.warning(str(e))
+    try:
+        with op.batch_alter_table('query') as batch_op:
+            batch_op.create_unique_constraint('client_id', ['client_id'])
+    except Exception as e:
+        logging.warning(str(e))
+
+    try:
+        with op.batch_alter_table('query') as batch_op:
+            batch_op.drop_column('name')
+    except Exception as e:
+        logging.warning(str(e))
+
+    try:
+        # wasn't created for some databases in the migration b4456560d4f3
+        if not table_has_constraint('tables', '_customer_location_uc', db):
+            with op.batch_alter_table('tables') as batch_op:
+                batch_op.create_unique_constraint(
+                    '_customer_location_uc',
+                    ['database_id', 'schema', 'table_name'])
+                batch_op.drop_index('table_name')
+    except Exception as e:
+        logging.warning(str(e))
+
+
+def downgrade():
+    try:
+        with op.batch_alter_table('tables') as batch_op:
+            batch_op.create_index('table_name', ['table_name'], unique=True)
+    except Exception as e:
+        logging.warning(str(e))
+
+    try:
+        with op.batch_alter_table("slices") as batch_op:
+            batch_op.add_column(sa.Column(
+                'table_id', mysql.INTEGER(display_width=11),
+                autoincrement=False, nullable=True))
+            batch_op.add_column(sa.Column(
+                'druid_datasource_id', sa.Integer(), autoincrement=False,
+                nullable=True))
+            batch_op.create_foreign_key(
+                'slices_ibfk_1', 'datasources', ['druid_datasource_id'],
+                ['id'])
+            batch_op.create_foreign_key(
+                'slices_ibfk_2', 'tables', ['table_id'], ['id'])
+    except Exception as e:
+        logging.warning(str(e))
+
+    try:
+        fk_columns = generic_find_constraint_name(
+            table='columns', columns={'datasource_name'},
+            referenced='datasources', db=db)
+        with op.batch_alter_table("columns") as batch_op:
+            batch_op.drop_constraint(fk_columns, type_="foreignkey")
+    except Exception as e:
+        logging.warning(str(e))
+
+    op.add_column(
+        'query', sa.Column('name', sa.String(length=256), nullable=True))
+    try:
+        with op.batch_alter_table("query") as batch_op:
+            batch_op.drop_constraint('client_id', type_='unique')
+    except Exception as e:
+        logging.warning(str(e))

caravel/models.py

@@ -1943,7 +1943,7 @@ class Query(Model):
 
     __tablename__ = 'query'
     id = Column(Integer, primary_key=True)
-    client_id = Column(String(11), unique=True)
+    client_id = Column(String(11), unique=True, nullable=False)
 
     database_id = Column(Integer, ForeignKey('dbs.id'), nullable=False)
 

caravel/utils.py

@@ -446,6 +446,16 @@ def validate_json(obj):
             raise CaravelException("JSON is not valid")
 
 
+def table_has_constraint(table, name, db):
+    """Utility to find a constraint name in alembic migrations"""
+    t = sa.Table(table, db.metadata, autoload=True, autoload_with=db.engine)
+
+    for c in t.constraints:
+        if c.name == name:
+            return True
+    return False
+
+
 class timeout(object):
     """
     To be used in a ``with`` block and timeout its content.

tests/celery_tests.py

@@ -125,7 +125,7 @@ def tearDownClass(cls):
             shell=True
         )
 
-    def run_sql(self, dbid, sql, cta='false', tmp_table='tmp',
+    def run_sql(self, dbid, sql, client_id, cta='false', tmp_table='tmp',
                 async='false'):
         self.login()
         resp = self.client.post(
@@ -136,7 +136,7 @@ def run_sql(self, dbid, sql, cta='false', tmp_table='tmp',
                 async=async,
                 select_as_cta=cta,
                 tmp_table_name=tmp_table,
-                client_id="not_used",
+                client_id=client_id,
             ),
         )
         self.logout()
@@ -185,14 +185,14 @@ def test_run_sync_query(self):
         # Case 1.
         # Table doesn't exist.
         sql_dont_exist = 'SELECT name FROM table_dont_exist'
-        result1 = self.run_sql(1, sql_dont_exist, cta='true', )
+        result1 = self.run_sql(1, sql_dont_exist, "1", cta='true')
         self.assertTrue('error' in result1)
 
         # Case 2.
         # Table and DB exists, CTA call to the backend.
         sql_where = "SELECT name FROM ab_permission WHERE name='can_sql'"
         result2 = self.run_sql(
-            1, sql_where, tmp_table='tmp_table_2', cta='true')
+            1, sql_where, "2", tmp_table='tmp_table_2', cta='true')
         self.assertEqual(QueryStatus.SUCCESS, result2['query']['state'])
         self.assertEqual([], result2['data'])
         self.assertEqual([], result2['columns'])
@@ -207,7 +207,7 @@ def test_run_sync_query(self):
         # Table and DB exists, CTA call to the backend, no data.
         sql_empty_result = 'SELECT * FROM ab_user WHERE id=666'
         result3 = self.run_sql(
-            1, sql_empty_result, tmp_table='tmp_table_3', cta='true',)
+            1, sql_empty_result, "3", tmp_table='tmp_table_3', cta='true',)
         self.assertEqual(QueryStatus.SUCCESS, result3['query']['state'])
         self.assertEqual([], result3['data'])
         self.assertEqual([], result3['columns'])
@@ -226,7 +226,7 @@ def test_run_async_query(self):
         # Table and DB exists, async CTA call to the backend.
         sql_where = "SELECT name FROM ab_role WHERE name='Admin'"
         result1 = self.run_sql(
-            1, sql_where, async='true', tmp_table='tmp_async_1', cta='true')
+            1, sql_where, "4", async='true', tmp_table='tmp_async_1', cta='true')
         assert result1['query']['state'] in (
             QueryStatus.PENDING, QueryStatus.RUNNING, QueryStatus.SUCCESS)
 

tests/core_tests.py

@@ -572,7 +572,7 @@ def test_gamma(self):
         resp = self.client.get('/dashboardmodelview/list/')
         assert "List Dashboard" in resp.data.decode('utf-8')
 
-    def run_sql(self, sql, user_name, client_id='not_used'):
+    def run_sql(self, sql, user_name, client_id):
         self.login(username=user_name)
         dbid = (
             db.session.query(models.Database)
@@ -581,16 +581,17 @@ def run_sql(self, sql, user_name, client_id='not_used'):
         )
         resp = self.client.post(
             '/caravel/sql_json/',
-            data=dict(database_id=dbid, sql=sql, select_as_create_as=False, client_id=client_id),
+            data=dict(database_id=dbid, sql=sql, select_as_create_as=False,
+                      client_id=client_id),
         )
         self.logout()
         return json.loads(resp.data.decode('utf-8'))
 
     def test_sql_json(self):
-        data = self.run_sql('SELECT * FROM ab_user', 'admin')
+        data = self.run_sql('SELECT * FROM ab_user', 'admin', "1")
         assert len(data['data']) > 0
 
-        data = self.run_sql('SELECT * FROM unexistant_table', 'admin')
+        data = self.run_sql('SELECT * FROM unexistant_table', 'admin', "2")
         assert len(data['error']) > 0
 
     def test_sql_json_has_access(self):
@@ -617,7 +618,7 @@ def test_sql_json_has_access(self):
                 'gagarin', 'Iurii', 'Gagarin', '[email protected]',
                 appbuilder.sm.find_role('Astronaut'),
                 password='general')
-        data = self.run_sql('SELECT * FROM ab_user', 'gagarin')
+        data = self.run_sql('SELECT * FROM ab_user', 'gagarin', "3")
         db.session.query(models.Query).delete()
         db.session.commit()
         assert len(data['data']) > 0