Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring DB in sync with the models.py #1172

Merged
merged 2 commits into from
Sep 28, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 111 additions & 0 deletions caravel/migrations/versions/3b626e2a6783_sync_db_with_models.py
Original file line number Diff line number Diff line change
@@ -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))
2 changes: 1 addition & 1 deletion caravel/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
10 changes: 10 additions & 0 deletions caravel/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 6 additions & 6 deletions tests/celery_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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()
Expand Down Expand Up @@ -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'])
Expand All @@ -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'])
Expand All @@ -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)

Expand Down
11 changes: 6 additions & 5 deletions tests/core_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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):
Expand All @@ -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
Expand Down