You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by er...@apache.org on 2019/10/09 18:39:18 UTC

[incubator-superset] branch master updated: Revert "[fix] make datasource names non-nullable (#8332)" (#8363)

This is an automated email from the ASF dual-hosted git repository.

erikrit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 7e7ea3d  Revert "[fix] make datasource names non-nullable (#8332)" (#8363)
7e7ea3d is described below

commit 7e7ea3d9a0b5a630d752c0ffce60a6e1fa5b4185
Author: serenajiang <se...@berkeley.edu>
AuthorDate: Wed Oct 9 11:38:57 2019 -0700

    Revert "[fix] make datasource names non-nullable (#8332)" (#8363)
    
    This reverts commit 65a05ca47e49c1ce30e7bd734b3e721c5c2fc0da.
---
 UPDATING.md                                        |  4 --
 superset/connectors/druid/models.py                |  2 +-
 superset/connectors/sqla/models.py                 |  2 +-
 ...a807eac07_make_datasource_names_non_nullable.py | 49 ----------------------
 superset/models/core.py                            |  4 +-
 tests/core_tests.py                                |  4 +-
 tests/db_engine_specs_test.py                      |  2 +-
 tests/model_tests.py                               | 12 +++---
 tests/sqla_models_tests.py                         | 12 +-----
 9 files changed, 14 insertions(+), 77 deletions(-)

diff --git a/UPDATING.md b/UPDATING.md
index 56b6013..55923ec 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -36,10 +36,6 @@ using `ENABLE_PROXY_FIX = True`, review the newly-introducted variable,
 backend serialization. To disable set `RESULTS_BACKEND_USE_MSGPACK = False`
 in your configuration.
 
-* [8332](https://github.com/apache/incubator-superset/pull/8332): makes
-`tables.table_name`, `dbs.database_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
diff --git a/superset/connectors/druid/models.py b/superset/connectors/druid/models.py
index 6c129f3..42de424 100644
--- a/superset/connectors/druid/models.py
+++ b/superset/connectors/druid/models.py
@@ -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, nullable=False)
+    cluster_name = Column(String(250), unique=True)
     broker_host = Column(String(255))
     broker_port = Column(Integer, default=8082)
     broker_endpoint = Column(String(255), default="druid/v2")
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index 054fa2b..980e607 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -306,7 +306,7 @@ class SqlaTable(Model, BaseDatasource):
     __tablename__ = "tables"
     __table_args__ = (UniqueConstraint("database_id", "table_name"),)
 
-    table_name = Column(String(250), nullable=False)
+    table_name = Column(String(250))
     main_dttm_col = Column(String(250))
     database_id = Column(Integer, ForeignKey("dbs.id"), nullable=False)
     fetch_values_predicate = Column(String(1000))
diff --git a/superset/migrations/versions/b6fa807eac07_make_datasource_names_non_nullable.py b/superset/migrations/versions/b6fa807eac07_make_datasource_names_non_nullable.py
deleted file mode 100644
index f0b7d6a..0000000
--- a/superset/migrations/versions/b6fa807eac07_make_datasource_names_non_nullable.py
+++ /dev/null
@@ -1,49 +0,0 @@
-# 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_datasource_names_non_nullable
-
-Revision ID: b6fa807eac07
-Revises: 258b5280a45e
-Create Date: 2019-10-02 00:29:16.679272
-
-"""
-from alembic import op
-from sqlalchemy import String
-
-# revision identifiers, used by Alembic.
-revision = "b6fa807eac07"
-down_revision = "258b5280a45e"
-
-
-def upgrade():
-    with op.batch_alter_table("clusters") as batch_op:
-        batch_op.alter_column("cluster_name", existing_type=String(250), nullable=False)
-    with op.batch_alter_table("dbs") as batch_op:
-        batch_op.alter_column(
-            "database_name", existing_type=String(250), nullable=False
-        )
-    with op.batch_alter_table("tables") as batch_op:
-        batch_op.alter_column("table_name", existing_type=String(250), nullable=False)
-
-
-def downgrade():
-    with op.batch_alter_table("clusters") as batch_op:
-        batch_op.alter_column("cluster_name", existing_type=String(250), nullable=True)
-    with op.batch_alter_table("dbs") as batch_op:
-        batch_op.alter_column("database_name", existing_type=String(250), nullable=True)
-    with op.batch_alter_table("tables") as batch_op:
-        batch_op.alter_column("table_name", existing_type=String(250), nullable=True)
diff --git a/superset/models/core.py b/superset/models/core.py
index 34d918f..200af49 100755
--- a/superset/models/core.py
+++ b/superset/models/core.py
@@ -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, nullable=False)
+    database_name = Column(String(250), unique=True)
     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.name
+        return self.verbose_name if self.verbose_name else self.database_name
 
     @property
     def name(self):
diff --git a/tests/core_tests.py b/tests/core_tests.py
index efefbad..3c2e8ea 100644
--- a/tests/core_tests.py
+++ b/tests/core_tests.py
@@ -684,9 +684,7 @@ 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(
-            table_name="test_comments_in_sqlatable_query_table", sql=commented_query
-        )
+        table = SqlaTable(sql=commented_query)
         rendered_query = str(table.get_from_clause())
         self.assertEqual(clean_query, rendered_query)
 
diff --git a/tests/db_engine_specs_test.py b/tests/db_engine_specs_test.py
index 4d624a3..50823bf 100644
--- a/tests/db_engine_specs_test.py
+++ b/tests/db_engine_specs_test.py
@@ -158,7 +158,7 @@ class DbEngineSpecsTestCase(SupersetTestCase):
         )
 
     def get_generic_database(self):
-        return Database(database_name="test_database", sqlalchemy_uri="sqlite://")
+        return Database(sqlalchemy_uri="sqlite://")
 
     def sql_limit_regex(
         self, sql, expected_sql, engine_spec_class=MySQLEngineSpec, limit=1000
diff --git a/tests/model_tests.py b/tests/model_tests.py
index 784c056..5033981 100644
--- a/tests/model_tests.py
+++ b/tests/model_tests.py
@@ -32,7 +32,7 @@ class DatabaseModelTestCase(SupersetTestCase):
     )
     def test_database_schema_presto(self):
         sqlalchemy_uri = "presto://presto.airbnb.io:8080/hive/default"
-        model = Database(database_name="test_database", sqlalchemy_uri=sqlalchemy_uri)
+        model = 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(database_name="test_database", sqlalchemy_uri=sqlalchemy_uri)
+        model = 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(database_name="test_database", sqlalchemy_uri=sqlalchemy_uri)
+        model = 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(database_name="test_database", sqlalchemy_uri=sqlalchemy_uri)
+        model = 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(database_name="test_database", sqlalchemy_uri=sqlalchemy_uri)
+        model = 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(database_name="test_database", sqlalchemy_uri=uri)
+        model = Database(sqlalchemy_uri=uri)
 
         model.impersonate_user = True
         user_name = make_url(model.get_sqla_engine(user_name=example_user).url).username
diff --git a/tests/sqla_models_tests.py b/tests/sqla_models_tests.py
index 8215dfe..326311a 100644
--- a/tests/sqla_models_tests.py
+++ b/tests/sqla_models_tests.py
@@ -43,11 +43,7 @@ class DatabaseModelTestCase(SupersetTestCase):
 
     def test_has_extra_cache_keys(self):
         query = "SELECT '{{ cache_key_wrapper('user_1') }}' as user"
-        table = SqlaTable(
-            table_name="test_has_extra_cache_keys_table",
-            sql=query,
-            database=get_example_database(),
-        )
+        table = SqlaTable(sql=query, database=get_example_database())
         query_obj = {
             "granularity": None,
             "from_dttm": None,
@@ -64,11 +60,7 @@ class DatabaseModelTestCase(SupersetTestCase):
 
     def test_has_no_extra_cache_keys(self):
         query = "SELECT 'abc' as user"
-        table = SqlaTable(
-            table_name="test_has_no_extra_cache_keys_table",
-            sql=query,
-            database=get_example_database(),
-        )
+        table = SqlaTable(sql=query, database=get_example_database())
         query_obj = {
             "granularity": None,
             "from_dttm": None,