villebro commented on a change in pull request #10593:
URL:
https://github.com/apache/incubator-superset/pull/10593#discussion_r475249487
##########
File path: requirements/testing.txt
##########
@@ -8,23 +8,39 @@
-r base.txt
-r integration.txt
-e file:. # via -r requirements/base.in
+appnope==0.1.0 # via ipython
astroid==2.4.2 # via pylint
+backcall==0.2.0 # via ipython
coverage==5.2.1 # via pytest-cov
+docker==4.3.0 # via -r requirements/testing.in
flask-testing==0.8.0 # via -r requirements/testing.in
iniconfig==1.0.1 # via pytest
+ipdb==0.13.3 # via -r requirements/testing.in
+ipython-genutils==0.2.0 # via traitlets
+ipython==7.16.1 # via -r requirements/testing.in, ipdb
Review comment:
This is surprising, do we need this dependency?
##########
File path: .github/workflows/superset-python.yml
##########
@@ -152,6 +152,63 @@ jobs:
run: |
bash <(curl -s https://codecov.io/bash) -cF python
+ test-postgres-hive:
+ runs-on: ubuntu-18.04
+ strategy:
+ matrix:
+ # run unit tests in multiple version just for fun
+ python-version: [3.6, 3.7, 3.8]
Review comment:
I'd suggest dropping 3.6 from new tests going forward.
##########
File path: tests/csv_upload_tests.py
##########
@@ -241,16 +279,29 @@ def test_import_csv(setup_csv_upload, create_csv_files):
# make sure that john and empty string are replaced with None
engine = get_upload_db().get_sqla_engine()
data = engine.execute(f"SELECT * from {CSV_UPLOAD_TABLE}").fetchall()
- assert data == [(None, 1, "x"), ("paul", 2, None)]
+ if utils.backend() == "hive":
+ # Be aware that hive only uses first value from the null values list
+ # it can be fixes if we will process csv file on the superset
webserver.
+ assert data == [("john", 1, "x"), ("paul", 2, None)]
+ else:
+ assert data == [(None, 1, "x"), ("paul", 2, None)]
Review comment:
I didn't quite understand this, why is "john" showing up here on Hive
when other dbs return `None`?
##########
File path: tests/model_tests.py
##########
@@ -111,44 +111,41 @@ def test_select_star(self):
db = get_example_database()
table_name = "energy_usage"
sql = db.select_star(table_name, show_cols=False,
latest_partition=False)
+ quote =
db.inspector.engine.dialect.identifier_preparer.quote_identifier
expected = (
textwrap.dedent(
f"""\
SELECT *
- FROM {table_name}
+ FROM {quote(table_name)}
LIMIT 100"""
)
- if db.backend != "presto"
+ if db.backend in {"presto", "hive"}
else textwrap.dedent(
f"""\
SELECT *
- FROM "{table_name}"
+ FROM {table_name}
LIMIT 100"""
)
)
assert expected in sql
sql = db.select_star(table_name, show_cols=True,
latest_partition=False)
- expected = (
- textwrap.dedent(
- f"""\
- SELECT source,
- target,
- value
- FROM {table_name}
- LIMIT 100"""
+ # TODO(bkyryliuk): unify sql generation
+ if db.backend == "presto":
+ assert (
+ 'SELECT "source" AS "source",\n "target" AS "target",\n
"value" AS "value"\nFROM "energy_usage"\nLIMIT 100'
+ == sql
)
- if db.backend != "presto"
- else textwrap.dedent(
- f"""\
- SELECT "source" AS "source",
- "target" AS "target",
- "value" AS "value"
- FROM "{table_name}"
- LIMIT 100"""
+ elif db.backend == "hive":
+ assert (
+ "SELECT `source`,\n `target`,\n `value`\nFROM
`energy_usage`\nLIMIT 100"
+ == sql
+ )
+ else:
+ assert (
+ "SELECT source,\n target,\n value\nFROM
energy_usage\nLIMIT 100"
+ in sql
Review comment:
nit/personal preference: I kind of preferred the appearance of
`textwrap.dedent` over this.
##########
File path: tests/db_engine_specs/oracle_tests.py
##########
@@ -18,6 +18,7 @@
from sqlalchemy.dialects import oracle
from sqlalchemy.dialects.oracle import DATE, NVARCHAR, VARCHAR
+from superset.db_engine_specs.hive import HiveEngineSpec
Review comment:
I assume this isn't needed here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
