bkyryliuk commented on a change in pull request #10593:
URL:
https://github.com/apache/incubator-superset/pull/10593#discussion_r471133110
##########
File path: tests/conftest.py
##########
@@ -14,18 +14,27 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+# isort:skip_file
Review comment:
from superset import db needs to be called after test_app import
e.g.
https://github.com/apache/incubator-superset/blob/fd2d1c58c566d9312d6cfc5641a06ac2b03e753a/tests/sqllab_tests.py#L27
##########
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 for ipdb, it CI had issues locationg ipython > 7.16.1, I'll try
it again.
As for ipdb - I like using it inside tox, that's why added to the test
dependencies. It provides really nice debugging experience.
##########
File path: requirements/testing.in
##########
@@ -16,7 +16,10 @@
#
-r base.in
-r integration.in
+ipdb
+ipython==7.16.1
Review comment:
Added comment:
```
# pinning ipython as pip-compile-multi was bringing higher version
# of the ipython that was not found in CI
```
##########
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:
good catch :)
##########
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:
sure thing
##########
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:
see the comment above, hive has it's own csv upload implementation and
only supports a single null value where as default implementation works with
the list. [ hive engine limitation ]
It can manually process csv to make it feature complete. Updated the comment
to be more descriptive & added a todo
----------------------------------------------------------------
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]
