From 0ba9aac55cc936df35aa3493084d05e3cbf7e7a3 Mon Sep 17 00:00:00 2001 From: Will Barrett Date: Mon, 2 Dec 2019 16:11:44 -0800 Subject: [PATCH 1/6] Require the SQLAlchemy URI when creating a database --- superset/models/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/models/core.py b/superset/models/core.py index 4136d7433c47a..c3719db60fc8f 100755 --- a/superset/models/core.py +++ b/superset/models/core.py @@ -753,7 +753,7 @@ class Database(Model, AuditMixinNullable, ImportMixin): verbose_name = Column(String(250), unique=True) # short unique name, used in permissions database_name = Column(String(250), unique=True, nullable=False) - sqlalchemy_uri = Column(String(1024)) + sqlalchemy_uri = Column(String(1024), nullable=False) password = Column(EncryptedType(String(1024), config["SECRET_KEY"])) cache_timeout = Column(Integer) select_as_create_table_as = Column(Boolean, default=False) From 8cdb968642a34b58f6ae15434c16cb28306e29a4 Mon Sep 17 00:00:00 2001 From: Will Barrett Date: Tue, 3 Dec 2019 10:30:44 -0800 Subject: [PATCH 2/6] Add migration to make dbs.sqlalchemy_uri not-nullable --- ...09d0_add_not_null_to_dbs_sqlalchemy_url.py | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100644 superset/migrations/versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py diff --git a/superset/migrations/versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py b/superset/migrations/versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py new file mode 100644 index 0000000000000..f92b707503052 --- /dev/null +++ b/superset/migrations/versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py @@ -0,0 +1,42 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""add_not_null_to_dbs_sqlalchemy_url + +Revision ID: 817e1c9b09d0 +Revises: db4b49eb0782 +Create Date: 2019-12-03 10:24:16.201580 + +""" +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = '817e1c9b09d0' +down_revision = 'db4b49eb0782' + + +def upgrade(): + with op.batch_alter_table('dbs') as batch_op: + batch_op.alter_column('sqlalchemy_uri', + existing_type=sa.VARCHAR(length=1024), + nullable=False) + +def downgrade(): + with op.batch_alter_table('dbs') as batch_op: + batch_op.alter_column('sqlalchemy_uri', + existing_type=sa.VARCHAR(length=1024), + nullable=True) From ead0c106d7a0df8a83ffe1f3f67df03c8966a129 Mon Sep 17 00:00:00 2001 From: Will Barrett Date: Tue, 3 Dec 2019 13:43:55 -0800 Subject: [PATCH 3/6] Fixes for black, isort, tests --- ...09d0_add_not_null_to_dbs_sqlalchemy_url.py | 23 ++++++++++--------- tests/base_tests.py | 1 + 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/superset/migrations/versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py b/superset/migrations/versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py index f92b707503052..c3b1d05b29c61 100644 --- a/superset/migrations/versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py +++ b/superset/migrations/versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py @@ -21,22 +21,23 @@ Create Date: 2019-12-03 10:24:16.201580 """ -from alembic import op import sqlalchemy as sa +from alembic import op # revision identifiers, used by Alembic. -revision = '817e1c9b09d0' -down_revision = 'db4b49eb0782' +revision = "817e1c9b09d0" +down_revision = "db4b49eb0782" def upgrade(): - with op.batch_alter_table('dbs') as batch_op: - batch_op.alter_column('sqlalchemy_uri', - existing_type=sa.VARCHAR(length=1024), - nullable=False) + with op.batch_alter_table("dbs") as batch_op: + batch_op.alter_column( + "sqlalchemy_uri", existing_type=sa.VARCHAR(length=1024), nullable=False + ) + def downgrade(): - with op.batch_alter_table('dbs') as batch_op: - batch_op.alter_column('sqlalchemy_uri', - existing_type=sa.VARCHAR(length=1024), - nullable=True) + with op.batch_alter_table("dbs") as batch_op: + batch_op.alter_column( + "sqlalchemy_uri", existing_type=sa.VARCHAR(length=1024), nullable=True + ) diff --git a/tests/base_tests.py b/tests/base_tests.py index 7399774cc8ef8..7120dae0c4718 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -226,6 +226,7 @@ def create_fake_db(self): cls=models.Database, criteria={"database_name": database_name}, session=db.session, + sqlalchemy_uri="sqlite://test", id=db_id, extra=extra, ) From 1ad9d4e1ed3f0694786d880c0229d650ed1bb3ed Mon Sep 17 00:00:00 2001 From: Will Barrett Date: Tue, 10 Dec 2019 12:51:54 -0800 Subject: [PATCH 4/6] Alter migration to use current revision from master as downgrade target --- .../versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset/migrations/versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py b/superset/migrations/versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py index c3b1d05b29c61..fc1a5aab67110 100644 --- a/superset/migrations/versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py +++ b/superset/migrations/versions/817e1c9b09d0_add_not_null_to_dbs_sqlalchemy_url.py @@ -26,7 +26,7 @@ # revision identifiers, used by Alembic. revision = "817e1c9b09d0" -down_revision = "db4b49eb0782" +down_revision = "89115a40e8ea" def upgrade(): From 8f6835b4e79f74cbfd68355337201bb9bde45e08 Mon Sep 17 00:00:00 2001 From: Will Barrett Date: Tue, 10 Dec 2019 14:36:03 -0800 Subject: [PATCH 5/6] Update tests to support new db constraint --- tests/security_tests.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/security_tests.py b/tests/security_tests.py index f5fbb1c80ce53..9f49de9ad070c 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -316,7 +316,7 @@ def test_set_perm_druid_cluster(self): def test_set_perm_database(self): session = db.session - database = Database(database_name="tmp_database") + database = Database(database_name="tmp_database", sqlalchemy_uri="sqlite://test") session.add(database) stored_db = ( @@ -346,7 +346,7 @@ def test_set_perm_database(self): def test_set_perm_slice(self): session = db.session - database = Database(database_name="tmp_database") + database = Database(database_name="tmp_database", sqlalchemy_uri="sqlite://test") table = SqlaTable(table_name="tmp_perm_table", database=database) session.add(database) session.add(table) From 1aa94dc2808d1cb65d9376c4394ca8bd22f1a41d Mon Sep 17 00:00:00 2001 From: Will Barrett Date: Wed, 11 Dec 2019 10:17:01 -0800 Subject: [PATCH 6/6] black --- tests/security_tests.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/security_tests.py b/tests/security_tests.py index 9f49de9ad070c..70a7337c5d7cf 100644 --- a/tests/security_tests.py +++ b/tests/security_tests.py @@ -316,7 +316,9 @@ def test_set_perm_druid_cluster(self): def test_set_perm_database(self): session = db.session - database = Database(database_name="tmp_database", sqlalchemy_uri="sqlite://test") + database = Database( + database_name="tmp_database", sqlalchemy_uri="sqlite://test" + ) session.add(database) stored_db = ( @@ -346,7 +348,9 @@ def test_set_perm_database(self): def test_set_perm_slice(self): session = db.session - database = Database(database_name="tmp_database", sqlalchemy_uri="sqlite://test") + database = Database( + database_name="tmp_database", sqlalchemy_uri="sqlite://test" + ) table = SqlaTable(table_name="tmp_perm_table", database=database) session.add(database) session.add(table)