diff --git a/caravel/migrations/versions/3b626e2a6783_sync_db_with_models.py b/caravel/migrations/versions/3b626e2a6783_sync_db_with_models.py new file mode 100644 index 000000000000..1afecfd8803a --- /dev/null +++ b/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)) diff --git a/caravel/models.py b/caravel/models.py index 05b334a6b21b..c99c4a534212 100644 --- a/caravel/models.py +++ b/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) diff --git a/caravel/utils.py b/caravel/utils.py index 3c298c780f2c..168656986f0a 100644 --- a/caravel/utils.py +++ b/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. diff --git a/tests/celery_tests.py b/tests/celery_tests.py index 46cef8f3b826..b973ae556e58 100644 --- a/tests/celery_tests.py +++ b/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) diff --git a/tests/core_tests.py b/tests/core_tests.py index af90591e31f7..cf5ecd0ad97e 100644 --- a/tests/core_tests.py +++ b/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', 'gagarin@cosmos.ussr', 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