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

@@ -38,6 +38,7 @@ changelog.sh
 dist
 dump.rdb
 env
+venv*
 env_py3
 envpy3
 env36

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

setup.py

@@ -113,12 +113,15 @@ setup(
             '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',

superset/db_engine_specs/mysql.py

@@ -67,7 +67,7 @@ class MySQLEngineSpec(BaseEngineSpec):
     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

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)

superset/sql_parse.py

@@ -193,7 +193,7 @@ class ParsedQuery(object):
         """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

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 @@ class SupersetTestCase(unittest.TestCase):
             .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:

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 @@ class DbEngineSpecsTestCase(SupersetTestCase):
             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 @@ class DbEngineSpecsTestCase(SupersetTestCase):
     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 @@ class DbEngineSpecsTestCase(SupersetTestCase):
                 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 @@ class DbEngineSpecsTestCase(SupersetTestCase):
 
     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):

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 @@ class DatabaseModelTestCase(SupersetTestCase):
         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 @@ class DatabaseModelTestCase(SupersetTestCase):
         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 @@ class DatabaseModelTestCase(SupersetTestCase):
         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'

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'