Skip to content

Commit

Permalink
Revert "[fix] make datasource names non-nullable (#8332)" (#8363)
Browse files Browse the repository at this point in the history
This reverts commit 65a05ca.
  • Loading branch information
serenajiang authored and Erik Ritter committed Oct 9, 2019
1 parent 8b85a8f commit 7e7ea3d
Show file tree
Hide file tree
Showing 9 changed files with 14 additions and 77 deletions.
4 changes: 0 additions & 4 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion superset/connectors/druid/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down

This file was deleted.

4 changes: 2 additions & 2 deletions superset/models/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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):
Expand Down
4 changes: 1 addition & 3 deletions tests/core_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -684,9 +684,7 @@ def test_mssql_engine_spec_odbc(self):
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)

Expand Down
2 changes: 1 addition & 1 deletion tests/db_engine_specs_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def test_hive_error_msg(self):
)

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
Expand Down
12 changes: 6 additions & 6 deletions tests/model_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -41,7 +41,7 @@ def test_database_schema_presto(self):
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)
Expand All @@ -51,7 +51,7 @@ def test_database_schema_presto(self):

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)
Expand All @@ -67,7 +67,7 @@ def test_database_schema_postgres(self):
)
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)

Expand All @@ -79,7 +79,7 @@ def test_database_schema_hive(self):
)
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)
Expand All @@ -93,7 +93,7 @@ def test_database_schema_mysql(self):
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
Expand Down
12 changes: 2 additions & 10 deletions tests/sqla_models_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,7 @@ def test_is_time_by_type(self):

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,
Expand All @@ -64,11 +60,7 @@ def test_has_extra_cache_keys(self):

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,
Expand Down

0 comments on commit 7e7ea3d

Please sign in to comment.