pt
/
superset
mirror of https://github.com/apache/superset.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[fix] make names non-nullable (#8371)

This commit is contained in:
serenajiang 2019-10-15 16:51:04 -07:00 committed by John Bodley
parent fcb39f9091
commit 876d329474
10 changed files with 174 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
UPDATING.md
View File

@ -36,6 +36,10 @@ using `ENABLE_PROXY_FIX = True`, review the newly-introducted variable,
backend serialization. To disable set `RESULTS_BACKEND_USE_MSGPACK = False`
in your configuration.
* [8371](https://github.com/apache/incubator-superset/pull/8371): makes
`tables.table_name`, `dbs.database_name`, `datasources.cluster_name`, and `clusters.cluster_name` non-nullable.
Depending on the integrity of the data, manual intervention may be required.
## 0.34.0
* [7848](https://github.com/apache/incubator-superset/pull/7848): If you are

6
superset/connectors/druid/models.py
View File

@ -128,7 +128,7 @@ class DruidCluster(Model, AuditMixinNullable, ImportMixin):
id = Column(Integer, primary_key=True)
verbose_name = Column(String(250), unique=True)
# short unique name, used in permissions
cluster_name = Column(String(250), unique=True)
cluster_name = Column(String(250), unique=True, nullable=False)
broker_host = Column(String(255))
broker_port = Column(Integer, default=8082)
broker_endpoint = Column(String(255), default="druid/v2")
@ -480,7 +480,9 @@ class DruidDatasource(Model, BaseDatasource):
is_hidden = Column(Boolean, default=False)
filter_select_enabled = Column(Boolean, default=True) # override default
fetch_values_from = Column(String(100))
cluster_name = Column(String(250), ForeignKey("clusters.cluster_name"))
cluster_name = Column(
String(250), ForeignKey("clusters.cluster_name"), nullable=False
)
cluster = relationship(
"DruidCluster", backref="datasources", foreign_keys=[cluster_name]
)

2
superset/connectors/sqla/models.py
View File

@ -306,7 +306,7 @@ class SqlaTable(Model, BaseDatasource):
__tablename__ = "tables"
__table_args__ = (UniqueConstraint("database_id", "table_name"),)
table_name = Column(String(250))
table_name = Column(String(250), nullable=False)
main_dttm_col = Column(String(250))
database_id = Column(Integer, ForeignKey("dbs.id"), nullable=False)
fetch_values_predicate = Column(String(1000))

123
superset/migrations/versions/b6fa807eac07_make_names_non_nullable.py Normal file
View File

@ -0,0 +1,123 @@
# 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.
"""make_names_non_nullable
Revision ID: b6fa807eac07
Revises: 1495eb914ad3
Create Date: 2019-10-02 00:29:16.679272
"""
from alembic import op
import sqlalchemy as sa
from superset.utils.core import generic_find_fk_constraint_name
# revision identifiers, used by Alembic.
revision = "b6fa807eac07"
down_revision = "1495eb914ad3"
conv = {
"fk": "fk_%(table_name)s_%(column_0_name)s_%(referred_table_name)s",
"uq": "uq_%(table_name)s_%(column_0_name)s",
}
def upgrade():
bind = op.get_bind()
insp = sa.engine.reflection.Inspector.from_engine(bind)
# First, drop the foreign key constraint prior to altering columns.
fk_datasources_cluster_name_clusters = (
generic_find_fk_constraint_name(
"datasources", {"cluster_name"}, "clusters", insp
)
or "fk_datasources_cluster_name_clusters"
)
with op.batch_alter_table("datasources", naming_convention=conv) as batch_op:
batch_op.drop_constraint(
fk_datasources_cluster_name_clusters, type_="foreignkey"
)
# Second, make the columns non-nullable.
with op.batch_alter_table("datasources") as batch_op:
batch_op.alter_column(
"cluster_name", existing_type=sa.String(250), nullable=False
)
with op.batch_alter_table("clusters") as batch_op:
batch_op.alter_column(
"cluster_name", existing_type=sa.String(250), nullable=False
)
with op.batch_alter_table("dbs") as batch_op:
batch_op.alter_column(
"database_name", existing_type=sa.String(250), nullable=False
)
with op.batch_alter_table("tables") as batch_op:
batch_op.alter_column(
"table_name", existing_type=sa.String(250), nullable=False
)
# Finally, re-add the foreign key constraint.
with op.batch_alter_table("datasources") as batch_op:
batch_op.create_foreign_key(
fk_datasources_cluster_name_clusters,
"clusters",
["cluster_name"],
["cluster_name"],
)
def downgrade():
bind = op.get_bind()
insp = sa.engine.reflection.Inspector.from_engine(bind)
# First, drop the foreign key constraint prior to altering columns.
fk_datasources_cluster_name_clusters = (
generic_find_fk_constraint_name(
"datasources", {"cluster_name"}, "clusters", insp
)
or "fk_datasources_cluster_name_clusters"
)
with op.batch_alter_table("datasources", naming_convention=conv) as batch_op:
batch_op.drop_constraint(
fk_datasources_cluster_name_clusters, type_="foreignkey"
)
# Second, make the columns nullable.
with op.batch_alter_table("datasources") as batch_op:
batch_op.alter_column(
"cluster_name", existing_type=sa.String(250), nullable=True
)
with op.batch_alter_table("clusters") as batch_op:
batch_op.alter_column(
"cluster_name", existing_type=sa.String(250), nullable=True
)
with op.batch_alter_table("dbs") as batch_op:
batch_op.alter_column(
"database_name", existing_type=sa.String(250), nullable=True
)
with op.batch_alter_table("tables") as batch_op:
batch_op.alter_column("table_name", existing_type=sa.String(250), nullable=True)
# Finally, re-add the foreign key constraint.
with op.batch_alter_table("datasources") as batch_op:
batch_op.create_foreign_key(
fk_datasources_cluster_name_clusters,
"clusters",
["cluster_name"],
["cluster_name"],
)

4
superset/models/core.py
View File

@ -723,7 +723,7 @@ class Database(Model, AuditMixinNullable, ImportMixin):
id = Column(Integer, primary_key=True)
verbose_name = Column(String(250), unique=True)
# short unique name, used in permissions
database_name = Column(String(250), unique=True)
database_name = Column(String(250), unique=True, nullable=False)
sqlalchemy_uri = Column(String(1024))
password = Column(EncryptedType(String(1024), config.get("SECRET_KEY")))
cache_timeout = Column(Integer)
@ -763,7 +763,7 @@ class Database(Model, AuditMixinNullable, ImportMixin):
export_children = ["tables"]
def __repr__(self):
return self.verbose_name if self.verbose_name else self.database_name
return self.name
@property
def name(self):

4
tests/core_tests.py
View File

@ -684,7 +684,9 @@ class CoreTests(SupersetTestCase):
def test_comments_in_sqlatable_query(self):
clean_query = "SELECT '/* val 1 */' as c1, '-- val 2' as c2 FROM tbl"
commented_query = "/* comment 1 */" + clean_query + "-- comment 2"
table = SqlaTable(sql=commented_query)
table = SqlaTable(
table_name="test_comments_in_sqlatable_query_table", sql=commented_query
)
rendered_query = str(table.get_from_clause())
self.assertEqual(clean_query, rendered_query)

2
tests/db_engine_specs_test.py
View File

@ -158,7 +158,7 @@ class DbEngineSpecsTestCase(SupersetTestCase):
)
def get_generic_database(self):
return Database(sqlalchemy_uri="sqlite://")
return Database(database_name="test_database", sqlalchemy_uri="sqlite://")
def sql_limit_regex(
self, sql, expected_sql, engine_spec_class=MySQLEngineSpec, limit=1000

22
tests/druid_tests.py
View File

@ -125,6 +125,13 @@ class DruidTests(SupersetTestCase):
.first()
)
if cluster:
for datasource in (
db.session.query(DruidDatasource)
.filter_by(cluster_name=cluster.cluster_name)
.all()
):
db.session.delete(datasource)
db.session.delete(cluster)
db.session.commit()
@ -297,13 +304,17 @@ class DruidTests(SupersetTestCase):
db.session.merge(cluster)
gamma_ds = self.get_or_create(
DruidDatasource, {"datasource_name": "datasource_for_gamma"}, db.session
DruidDatasource,
{"datasource_name": "datasource_for_gamma", "cluster": cluster},
db.session,
)
gamma_ds.cluster = cluster
db.session.merge(gamma_ds)
no_gamma_ds = self.get_or_create(
DruidDatasource, {"datasource_name": "datasource_not_for_gamma"}, db.session
DruidDatasource,
{"datasource_name": "datasource_not_for_gamma", "cluster": cluster},
db.session,
)
no_gamma_ds.cluster = cluster
db.session.merge(no_gamma_ds)
@ -340,6 +351,13 @@ class DruidTests(SupersetTestCase):
.first()
)
if cluster:
for datasource in (
db.session.query(DruidDatasource)
.filter_by(cluster_name=cluster.cluster_name)
.all()
):
db.session.delete(datasource)
db.session.delete(cluster)
db.session.commit()

12
tests/model_tests.py
View File

@ -32,7 +32,7 @@ class DatabaseModelTestCase(SupersetTestCase):
)
def test_database_schema_presto(self):
sqlalchemy_uri = "presto://presto.airbnb.io:8080/hive/default"
model = Database(sqlalchemy_uri=sqlalchemy_uri)
model = Database(database_name="test_database", sqlalchemy_uri=sqlalchemy_uri)
db = make_url(model.get_sqla_engine().url).database
self.assertEquals("hive/default", db)
@ -41,7 +41,7 @@ class DatabaseModelTestCase(SupersetTestCase):
self.assertEquals("hive/core_db", db)
sqlalchemy_uri = "presto://presto.airbnb.io:8080/hive"
model = Database(sqlalchemy_uri=sqlalchemy_uri)
model = Database(database_name="test_database", sqlalchemy_uri=sqlalchemy_uri)
db = make_url(model.get_sqla_engine().url).database
self.assertEquals("hive", db)
@ -51,7 +51,7 @@ class DatabaseModelTestCase(SupersetTestCase):
def test_database_schema_postgres(self):
sqlalchemy_uri = "postgresql+psycopg2://postgres.airbnb.io:5439/prod"
model = Database(sqlalchemy_uri=sqlalchemy_uri)
model = Database(database_name="test_database", sqlalchemy_uri=sqlalchemy_uri)
db = make_url(model.get_sqla_engine().url).database
self.assertEquals("prod", db)
@ -67,7 +67,7 @@ class DatabaseModelTestCase(SupersetTestCase):
)
def test_database_schema_hive(self):
sqlalchemy_uri = "hive://hive@hive.airbnb.io:10000/default?auth=NOSASL"
model = Database(sqlalchemy_uri=sqlalchemy_uri)
model = Database(database_name="test_database", sqlalchemy_uri=sqlalchemy_uri)
db = make_url(model.get_sqla_engine().url).database
self.assertEquals("default", db)
@ -79,7 +79,7 @@ class DatabaseModelTestCase(SupersetTestCase):
)
def test_database_schema_mysql(self):
sqlalchemy_uri = "mysql://root@localhost/superset"
model = Database(sqlalchemy_uri=sqlalchemy_uri)
model = Database(database_name="test_database", sqlalchemy_uri=sqlalchemy_uri)
db = make_url(model.get_sqla_engine().url).database
self.assertEquals("superset", db)
@ -93,7 +93,7 @@ class DatabaseModelTestCase(SupersetTestCase):
def test_database_impersonate_user(self):
uri = "mysql://root@localhost"
example_user = "giuseppe"
model = Database(sqlalchemy_uri=uri)
model = Database(database_name="test_database", sqlalchemy_uri=uri)
model.impersonate_user = True
user_name = make_url(model.get_sqla_engine(user_name=example_user).url).username

12
tests/sqla_models_tests.py
View File

@ -43,7 +43,11 @@ class DatabaseModelTestCase(SupersetTestCase):
def test_has_extra_cache_keys(self):
query = "SELECT '{{ cache_key_wrapper('user_1') }}' as user"
table = SqlaTable(sql=query, database=get_example_database())
table = SqlaTable(
table_name="test_has_extra_cache_keys_table",
sql=query,
database=get_example_database(),
)
query_obj = {
"granularity": None,
"from_dttm": None,
@ -60,7 +64,11 @@ class DatabaseModelTestCase(SupersetTestCase):
def test_has_no_extra_cache_keys(self):
query = "SELECT 'abc' as user"
table = SqlaTable(sql=query, database=get_example_database())
table = SqlaTable(
table_name="test_has_no_extra_cache_keys_table",
sql=query,
database=get_example_database(),
)
query_obj = {
"granularity": None,
"from_dttm": None,