From 859d6e7c6a8b89c7b3fd0687aaca51bd2868144a Mon Sep 17 00:00:00 2001 From: Maxime Beauchemin Date: Sun, 23 Jun 2019 22:37:41 -0700 Subject: [PATCH] chore: taking db dependencies out of requirements-dev.txt (#7605) * chore: taking db dependencies out of requirements-dev.txt The deps on mysqlclient and psycopg2 * Fix unit tests * fix tox.ini * fix tests --- .gitignore | 1 + requirements-dev.txt | 2 -- setup.py | 5 ++++- superset/db_engine_specs/mysql.py | 2 +- superset/db_engines/hive.py | 6 ++--- superset/sql_parse.py | 2 +- tests/base_tests.py | 9 ++++++++ tests/db_engine_specs_test.py | 37 +++++++++++++++---------------- tests/model_tests.py | 9 ++++++++ tox.ini | 12 ++++++++++ 10 files changed, 58 insertions(+), 27 deletions(-) diff --git a/.gitignore b/.gitignore index 2c553ad1db036..fcfe9cc16d126 100644 --- a/.gitignore +++ b/.gitignore @@ -38,6 +38,7 @@ changelog.sh dist dump.rdb env +venv* env_py3 envpy3 env36 diff --git a/requirements-dev.txt b/requirements-dev.txt index 7059084678669..1b008ad2664bb 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -23,7 +23,6 @@ flake8==3.7.7 flask-cors==3.0.7 ipdb==0.12 mypy==0.670 -mysqlclient==1.4.2.post1 nose==1.3.7 pip-tools==3.7.0 psycopg2-binary==2.7.5 @@ -33,5 +32,4 @@ pylint==1.9.2 python-dotenv==0.10.1 redis==2.10.6 statsd==3.3.0 -thrift==0.11.0 tox==3.11.1 diff --git a/setup.py b/setup.py index 28c6bbd41e564..8d3fcc6c9be64 100644 --- a/setup.py +++ b/setup.py @@ -113,12 +113,15 @@ def get_git_sha(): 'pandas_gbq>=0.10.0', ], 'cors': ['flask-cors>=2.0.0'], + 'gsheets': ['gsheetsdb>=0.1.9'], 'hive': [ 'pyhive[hive]>=0.6.1', 'tableschema', + 'thrift>=0.11.0, <1.0.0', ], + 'mysql': ['mysqlclient==1.4.2.post1'], + 'postgres': ['psycopg2-binary==2.7.5'], 'presto': ['pyhive[presto]>=0.4.0'], - 'gsheets': ['gsheetsdb>=0.1.9'], }, author='Apache Software Foundation', author_email='dev@superset.incubator.apache.org', diff --git a/superset/db_engine_specs/mysql.py b/superset/db_engine_specs/mysql.py index 31a6f4c99f839..5019407b20990 100644 --- a/superset/db_engine_specs/mysql.py +++ b/superset/db_engine_specs/mysql.py @@ -67,7 +67,7 @@ def adjust_database_uri(cls, uri, selected_schema=None): def get_datatype(cls, type_code): if not cls.type_code_map: # only import and store if needed at least once - import MySQLdb + import MySQLdb # pylint: disable=import-error ft = MySQLdb.constants.FIELD_TYPE cls.type_code_map = { getattr(ft, k): k diff --git a/superset/db_engines/hive.py b/superset/db_engines/hive.py index 63342577a253f..bab2e38dd981b 100644 --- a/superset/db_engines/hive.py +++ b/superset/db_engines/hive.py @@ -28,9 +28,9 @@ def fetch_logs(self, max_rows=1024, .. note:: This is not a part of DB-API. """ - from pyhive import hive - from TCLIService import ttypes - from thrift import Thrift + from pyhive import hive # noqa + from TCLIService import ttypes # noqa + from thrift import Thrift # pylint: disable=import-error orientation = orientation or ttypes.TFetchOrientation.FETCH_NEXT try: req = ttypes.TGetLogReq(operationHandle=self._operationHandle) diff --git a/superset/sql_parse.py b/superset/sql_parse.py index e2c25ef5011f4..868800b696678 100644 --- a/superset/sql_parse.py +++ b/superset/sql_parse.py @@ -193,7 +193,7 @@ def get_query_with_new_limit(self, new_limit): """returns the query with the specified limit""" """does not change the underlying query""" if not self._limit: - return f'{self.sql}\nLIMIT {new_limit}' + return f'{self.stripped()}\nLIMIT {new_limit}' limit_pos = None tokens = self._parsed[0].tokens # Add all items to before_str until there is a limit diff --git a/tests/base_tests.py b/tests/base_tests.py index b24193c1cb4ce..7ccb6373205b7 100644 --- a/tests/base_tests.py +++ b/tests/base_tests.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. """Unit tests for Superset""" +import imp import json import unittest from unittest.mock import Mock, patch @@ -73,6 +74,14 @@ def get_table(self, table_id): .one() ) + @staticmethod + def is_module_installed(module_name): + try: + imp.find_module(module_name) + return True + except ImportError: + return False + def get_or_create(self, cls, criteria, session, **kwargs): obj = session.query(cls).filter_by(**criteria).first() if not obj: diff --git a/tests/db_engine_specs_test.py b/tests/db_engine_specs_test.py index 6acc8bae7b781..e3e81bb290513 100644 --- a/tests/db_engine_specs_test.py +++ b/tests/db_engine_specs_test.py @@ -14,6 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +import unittest from unittest import mock from sqlalchemy import column, literal_column, select, table @@ -137,7 +138,7 @@ def test_hive_error_msg(self): HiveEngineSpec.extract_error_message(Exception(msg))) def get_generic_database(self): - return Database(sqlalchemy_uri='mysql://localhost') + return Database(sqlalchemy_uri='sqlite://') def sql_limit_regex( self, sql, expected_sql, @@ -177,22 +178,16 @@ def test_extract_limit_from_query(self, engine_spec_class=MySQLEngineSpec): def test_wrapped_query(self): self.sql_limit_regex( 'SELECT * FROM a', - 'SELECT * \nFROM (SELECT * FROM a) AS inner_qry \n LIMIT 1000', - MssqlEngineSpec, - ) - - def test_wrapped_semi(self): - self.sql_limit_regex( - 'SELECT * FROM a;', - 'SELECT * \nFROM (SELECT * FROM a) AS inner_qry \n LIMIT 1000', + 'SELECT * \nFROM (SELECT * FROM a) AS inner_qry\n LIMIT 1000 OFFSET 0', MssqlEngineSpec, ) + @unittest.skipUnless( + SupersetTestCase.is_module_installed('MySQLdb'), 'mysqlclient not installed') def test_wrapped_semi_tabs(self): self.sql_limit_regex( 'SELECT * FROM a \t \n ; \t \n ', - 'SELECT * \nFROM (SELECT * FROM a) AS inner_qry \n LIMIT 1000', - MssqlEngineSpec, + 'SELECT * FROM a\nLIMIT 1000', ) def test_simple_limit_query(self): @@ -247,10 +242,18 @@ def test_limit_expr_and_semicolon(self): LIMIT 1000""", ) - def test_get_datatype(self): - self.assertEquals('STRING', PrestoEngineSpec.get_datatype('string')) + @unittest.skipUnless( + SupersetTestCase.is_module_installed('MySQLdb'), 'mysqlclient not installed') + def test_get_datatype_mysql(self): self.assertEquals('TINY', MySQLEngineSpec.get_datatype(1)) self.assertEquals('VARCHAR', MySQLEngineSpec.get_datatype(15)) + + @unittest.skipUnless( + SupersetTestCase.is_module_installed('pyhive'), 'pyhive not installed') + def test_get_datatype_presto(self): + self.assertEquals('STRING', PrestoEngineSpec.get_datatype('string')) + + def test_get_datatype(self): self.assertEquals('VARCHAR', BaseEngineSpec.get_datatype('VARCHAR')) def test_limit_with_implicit_offset(self): @@ -291,12 +294,8 @@ def test_limit_with_explicit_offset(self): def test_limit_with_non_token_limit(self): self.sql_limit_regex( - """ - SELECT - 'LIMIT 777'""", - """ - SELECT - 'LIMIT 777'\nLIMIT 1000""", + """SELECT 'LIMIT 777'""", + """SELECT 'LIMIT 777'\nLIMIT 1000""", ) def test_time_grain_blacklist(self): diff --git a/tests/model_tests.py b/tests/model_tests.py index 53e53cc5f6516..578dd7805b3ab 100644 --- a/tests/model_tests.py +++ b/tests/model_tests.py @@ -15,6 +15,7 @@ # specific language governing permissions and limitations # under the License. import textwrap +import unittest import pandas from sqlalchemy.engine.url import make_url @@ -56,6 +57,10 @@ def test_database_schema_postgres(self): db = make_url(model.get_sqla_engine(schema='foo').url).database self.assertEquals('prod', db) + @unittest.skipUnless( + SupersetTestCase.is_module_installed('thrift'), 'thrift not installed') + @unittest.skipUnless( + SupersetTestCase.is_module_installed('pyhive'), 'pyhive not installed') def test_database_schema_hive(self): sqlalchemy_uri = 'hive://hive@hive.airbnb.io:10000/default?auth=NOSASL' model = Database(sqlalchemy_uri=sqlalchemy_uri) @@ -65,6 +70,8 @@ def test_database_schema_hive(self): db = make_url(model.get_sqla_engine(schema='core_db').url).database self.assertEquals('core_db', db) + @unittest.skipUnless( + SupersetTestCase.is_module_installed('MySQLdb'), 'mysqlclient not installed') def test_database_schema_mysql(self): sqlalchemy_uri = 'mysql://root@localhost/superset' model = Database(sqlalchemy_uri=sqlalchemy_uri) @@ -75,6 +82,8 @@ def test_database_schema_mysql(self): db = make_url(model.get_sqla_engine(schema='staging').url).database self.assertEquals('staging', db) + @unittest.skipUnless( + SupersetTestCase.is_module_installed('MySQLdb'), 'mysqlclient not installed') def test_database_impersonate_user(self): uri = 'mysql://root@localhost' example_user = 'giuseppe' diff --git a/tox.ini b/tox.ini index 8b39058da3df4..dc8f40dd85f3c 100644 --- a/tox.ini +++ b/tox.ini @@ -64,6 +64,18 @@ setenv = whitelist_externals = npm +[testenv:py36-mysql] +deps = + -rrequirements.txt + -rrequirements-dev.txt + .[mysql] + +[testenv:py36-postgres] +deps = + -rrequirements.txt + -rrequirements-dev.txt + .[postgres] + [testenv:cypress-dashboard] commands = npm install -g npm@'>=6.5.0'