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

Make schema name for the CTA queries and limit configurable #8867

Merged
merged 13 commits into from
Mar 3, 2020
Prev Previous commit
Next Next commit
Move tweaks to travis db setup
  • Loading branch information
bogdan-dbx committed Feb 26, 2020
commit 6e7dc8ce426302223b08832ded8a1488fb48c3ea
11 changes: 9 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ jobs:
before_script:
- mysql -u root -e "DROP DATABASE IF EXISTS superset; CREATE DATABASE superset DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci"
- mysql -u root -e "DROP DATABASE IF EXISTS sqllab_test_db; CREATE DATABASE sqllab_test_db DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s not clear from this PR why we need these additional two databases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@john-bodley this is purely for testing, e.g. there is a need to have 2 different schemas in the mysql & postgres to test the CTA behavior

- mysql -u root -e "DROP DATABASE IF EXISTS admin_database; CREATE DATABASE admin_database DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci"
- mysql -u root -e "CREATE USER 'mysqluser'@'localhost' IDENTIFIED BY 'mysqluserpassword';"
- mysql -u root -e "GRANT ALL ON *.* TO 'mysqluser'@'localhost';"
- language: python
Expand Down Expand Up @@ -92,9 +93,15 @@ jobs:
- postgresql
- redis-server
before_script:
- psql -U postgres -c "DROP DATABASE IF EXISTS superset; CREATE DATABASE superset;"
- psql -U postgres -c "DROP DATABASE IF EXISTS sqllab_test_db; CREATE DATABASE sqllab_test_db;"
- psql -U postgres -c "DROP DATABASE IF EXISTS superset;"
- psql -U postgres -c "CREATE DATABASE superset;"
- psql -U postgres superset -c "DROP SCHEMA IF EXISTS sqllab_test_db;"
- psql -U postgres superset -c "CREATE SCHEMA sqllab_test_db;"
- psql -U postgres superset -c "DROP SCHEMA IF EXISTS admin_database;"
- psql -U postgres superset -c "CREATE SCHEMA admin_database;"
- psql -U postgres -c "CREATE USER postgresuser WITH PASSWORD 'pguserpassword';"
- psql -U postgres superset -c "GRANT ALL PRIVILEGES ON SCHEMA sqllab_test_db to postgresuser";
- psql -U postgres superset -c "GRANT ALL PRIVILEGES ON SCHEMA admin_database to postgresuser";
- language: python
python: 3.6
env: TOXENV=pylint
Expand Down
42 changes: 24 additions & 18 deletions tests/celery_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,6 @@ def get_query_by_id(self, id):
@classmethod
def setUpClass(cls):
with app.app_context():
main_db = get_example_database()
# sqlite supports attaching schemas only for the single connection. It doesn't work with our celery setup.
# https://www.sqlite.org/inmemorydb.html
if main_db.backend != "sqlite":
db.session.execute("CREATE SCHEMA IF NOT EXISTS sqllab_test_db")

class CeleryConfig(object):
BROKER_URL = app.config["CELERY_CONFIG"].BROKER_URL
Expand All @@ -134,11 +129,6 @@ class CeleryConfig(object):

@classmethod
def tearDownClass(cls):
with app.app_context():
main_db = get_example_database()
if main_db.backend != "sqlite":
db.session.execute("DROP SCHEMA sqllab_test_db")

subprocess.call(
"ps auxww | grep 'celeryd' | awk '{print $2}' | xargs kill -9", shell=True
)
Expand Down Expand Up @@ -372,9 +362,12 @@ def test_default_data_serialization(self):
with mock.patch.object(
db_engine_spec, "expand_data", wraps=db_engine_spec.expand_data
) as expand_data:
data, selected_columns, all_columns, expanded_columns = sql_lab._serialize_and_expand_data(
results, db_engine_spec, False, True
)
(
data,
selected_columns,
all_columns,
expanded_columns,
) = sql_lab._serialize_and_expand_data(results, db_engine_spec, False, True)
expand_data.assert_called_once()

self.assertIsInstance(data, list)
Expand All @@ -393,9 +386,12 @@ def test_new_data_serialization(self):
with mock.patch.object(
db_engine_spec, "expand_data", wraps=db_engine_spec.expand_data
) as expand_data:
data, selected_columns, all_columns, expanded_columns = sql_lab._serialize_and_expand_data(
results, db_engine_spec, True
)
(
data,
selected_columns,
all_columns,
expanded_columns,
) = sql_lab._serialize_and_expand_data(results, db_engine_spec, True)
expand_data.assert_not_called()

self.assertIsInstance(data, bytes)
Expand All @@ -416,7 +412,12 @@ def test_default_payload_serialization(self):
"sql": "SELECT * FROM birth_names LIMIT 100",
"status": QueryStatus.PENDING,
}
serialized_data, selected_columns, all_columns, expanded_columns = sql_lab._serialize_and_expand_data(
(
serialized_data,
selected_columns,
all_columns,
expanded_columns,
) = sql_lab._serialize_and_expand_data(
results, db_engine_spec, use_new_deserialization
)
payload = {
Expand Down Expand Up @@ -449,7 +450,12 @@ def test_msgpack_payload_serialization(self):
"sql": "SELECT * FROM birth_names LIMIT 100",
"status": QueryStatus.PENDING,
}
serialized_data, selected_columns, all_columns, expanded_columns = sql_lab._serialize_and_expand_data(
(
serialized_data,
selected_columns,
all_columns,
expanded_columns,
) = sql_lab._serialize_and_expand_data(
results, db_engine_spec, use_new_deserialization
)
payload = {
Expand Down
7 changes: 0 additions & 7 deletions tests/sqllab_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ def test_sql_json_cta_dynamic_db(self):
if main_db.backend == "sqlite":
# sqlite doesn't support database creation
return
db.session.execute("CREATE SCHEMA IF NOT EXISTS admin_database")
db.session.commit()

old_allow_ctas = main_db.allow_ctas
main_db.allow_ctas = True # enable cta
Expand All @@ -100,11 +98,6 @@ def test_sql_json_cta_dynamic_db(self):

# cleanup
db.session.execute("DROP TABLE admin_database.test_target")

db.session.commit()
db.session.execute("DROP SCHEMA admin_database")
db.session.commit()

main_db.allow_ctas = old_allow_ctas
db.session.commit()

Expand Down