From 178ccf8da9101d66a22d5838af8c1b5f94b02b33 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 7 Jul 2021 14:17:13 -0700 Subject: [PATCH 1/4] feat: validate_parameters for GSheets --- requirements/base.txt | 60 +-- requirements/development.in | 1 + requirements/development.txt | 16 +- requirements/integration.txt | 8 - requirements/testing.in | 1 + requirements/testing.txt | 488 +++++++++++++++++- superset/db_engine_specs/gsheets.py | 55 +- tests/unit_tests/conftest.py | 30 ++ .../db_engine_specs/test_gsheets.py | 177 +++++++ 9 files changed, 773 insertions(+), 63 deletions(-) create mode 100644 tests/unit_tests/conftest.py create mode 100644 tests/unit_tests/db_engine_specs/test_gsheets.py diff --git a/requirements/base.txt b/requirements/base.txt index e6e19adadbb9d..ac968d9405359 100644 --- a/requirements/base.txt +++ b/requirements/base.txt @@ -68,6 +68,19 @@ dnspython==2.0.0 # via email-validator email-validator==1.1.1 # via flask-appbuilder +flask==1.1.2 + # via + # apache-superset + # flask-appbuilder + # flask-babel + # flask-caching + # flask-compress + # flask-jwt-extended + # flask-login + # flask-migrate + # flask-openid + # flask-sqlalchemy + # flask-wtf flask-appbuilder==3.3.0 # via apache-superset flask-babel==1.0.0 @@ -94,19 +107,6 @@ flask-wtf==0.14.3 # via # apache-superset # flask-appbuilder -flask==1.1.2 - # via - # apache-superset - # flask-appbuilder - # flask-babel - # flask-caching - # flask-compress - # flask-jwt-extended - # flask-login - # flask-migrate - # flask-openid - # flask-sqlalchemy - # flask-wtf geographiclib==1.50 # via geopy geopy==2.0.0 @@ -123,11 +123,6 @@ idna==2.10 # via # email-validator # yarl -importlib-metadata==2.1.1 - # via - # jsonschema - # kombu - # markdown isodate==0.6.0 # via apache-superset itsdangerous==1.1.0 @@ -154,15 +149,15 @@ markupsafe==1.1.1 # jinja2 # mako # wtforms -marshmallow-enum==1.5.1 - # via flask-appbuilder -marshmallow-sqlalchemy==0.23.1 - # via flask-appbuilder marshmallow==3.9.0 # via # flask-appbuilder # marshmallow-enum # marshmallow-sqlalchemy +marshmallow-enum==1.5.1 + # via flask-appbuilder +marshmallow-sqlalchemy==0.23.1 + # via flask-appbuilder msgpack==1.0.0 # via apache-superset multidict==5.0.0 @@ -176,7 +171,9 @@ numpy==1.19.4 # pandas # pyarrow packaging==20.4 - # via bleach + # via + # bleach + # deprecation pandas==1.2.2 # via apache-superset parsedatetime==2.6 @@ -264,10 +261,6 @@ six==1.15.0 # wtforms-json slackclient==2.5.0 # via apache-superset -sqlalchemy-utils==0.36.8 - # via - # apache-superset - # flask-appbuilder sqlalchemy==1.3.20 # via # alembic @@ -276,13 +269,16 @@ sqlalchemy==1.3.20 # flask-sqlalchemy # marshmallow-sqlalchemy # sqlalchemy-utils +sqlalchemy-utils==0.36.8 + # via + # apache-superset + # flask-appbuilder sqlparse==0.3.0 # via apache-superset typing-extensions==3.7.4.3 # via # aiohttp # apache-superset - # yarl urllib3==1.25.11 # via selenium vine==1.3.0 @@ -295,18 +291,16 @@ werkzeug==1.0.1 # via # flask # flask-jwt-extended -wtforms-json==0.3.3 - # via apache-superset wtforms==2.3.3 # via # flask-wtf # wtforms-json +wtforms-json==0.3.3 + # via apache-superset yarl==1.6.2 # via aiohttp zipp==3.4.1 - # via - # -r requirements/base.in - # importlib-metadata + # via -r requirements/base.in # The following packages are considered to be unsafe in a requirements file: # setuptools diff --git a/requirements/development.in b/requirements/development.in index 5ac06dc543399..1955141a30124 100644 --- a/requirements/development.in +++ b/requirements/development.in @@ -25,3 +25,4 @@ psycopg2-binary==2.8.5 tableschema thrift>=0.11.0,<1.0.0 pygithub>=1.54.1,<2.0.0 +pytest-mock>=3.5.1,<4 diff --git a/requirements/development.txt b/requirements/development.txt index 7b6ca3ffb7a53..e8eeeb364cb7d 100644 --- a/requirements/development.txt +++ b/requirements/development.txt @@ -1,4 +1,4 @@ -# SHA1:1b4d15a41f3498d2eb930ac3d3d4ce5d1f218a2f +# SHA1:6621b6d160d24719b26c9ccd5049ed48743c456d # # This file is autogenerated by pip-compile-multi # To update, run: @@ -28,6 +28,8 @@ future==0.18.2 # via pyhive ijson==3.1.2.post0 # via tabulator +iniconfig==1.1.1 + # via pytest jdcal==1.4.1 # via openpyxl jmespath==0.10.0 @@ -44,6 +46,8 @@ openpyxl==3.0.5 # via tabulator pillow==7.2.0 # via -r requirements/development.in +pluggy==0.13.1 + # via pytest psycopg2-binary==2.8.5 # via -r requirements/development.in pydruid==0.6.1 @@ -52,6 +56,10 @@ pygithub==1.54.1 # via -r requirements/development.in pyhive[hive]==0.6.3 # via -r requirements/development.in +pytest==6.2.4 + # via pytest-mock +pytest-mock==3.6.1 + # via -r requirements/development.in requests==2.24.0 # via # pydruid @@ -70,13 +78,15 @@ tableschema==1.20.0 # via -r requirements/development.in tabulator==1.52.5 # via tableschema -thrift-sasl==0.4.2 - # via pyhive thrift==0.13.0 # via # -r requirements/development.in # pyhive # thrift-sasl +thrift-sasl==0.4.2 + # via pyhive +toml==0.10.2 + # via pytest unicodecsv==0.14.1 # via # tableschema diff --git a/requirements/integration.txt b/requirements/integration.txt index e1cb0edb3e2a9..b9ec99d133268 100644 --- a/requirements/integration.txt +++ b/requirements/integration.txt @@ -21,12 +21,6 @@ filelock==3.0.12 # virtualenv identify==1.5.9 # via pre-commit -importlib-metadata==2.1.1 - # via - # pluggy - # pre-commit - # tox - # virtualenv nodeenv==1.5.0 # via pre-commit packaging==20.4 @@ -63,8 +57,6 @@ virtualenv==20.1.0 # via # pre-commit # tox -zipp==3.4.1 - # via importlib-metadata # The following packages are considered to be unsafe in a requirements file: # pip diff --git a/requirements/testing.in b/requirements/testing.in index e95cab68f04c7..d2f0b7011df31 100644 --- a/requirements/testing.in +++ b/requirements/testing.in @@ -32,3 +32,4 @@ pylint pytest pytest-cov statsd +pytest-mock>=3.5.1,<4 diff --git a/requirements/testing.txt b/requirements/testing.txt index 4d116c53dbddd..ef711bc7a6a18 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -1,84 +1,542 @@ -# SHA1:dba8c39bbdcb85b86e83af855d023b55a2674e87 -# -# This file is autogenerated by pip-compile-multi -# To update, run: -# -# pip-compile-multi -# --r development.txt --r integration.txt -e file:. # via -r requirements/base.in +aiohttp==3.7.2 + # via slackclient +alembic==1.4.3 + # via flask-migrate +amqp==2.6.1 + # via kombu +apispec[yaml]==3.3.2 + # via flask-appbuilder +appdirs==1.4.4 + # via virtualenv +appnope==0.1.2 + # via ipython astroid==2.4.2 # via pylint +async-timeout==3.0.1 + # via aiohttp +attrs==20.2.0 + # via + # aiohttp + # jsonschema + # pytest +babel==2.8.0 + # via flask-babel backcall==0.2.0 # via ipython +backoff==1.10.0 + # via apache-superset +billiard==3.6.3.0 + # via celery +bleach==3.3.0 + # via apache-superset +boto3==1.16.10 + # via tabulator +botocore==1.19.10 + # via + # boto3 + # s3transfer +brotli==1.0.9 + # via flask-compress +cached-property==1.5.2 + # via tableschema +cachelib==0.1.1 + # via apache-superset +celery==4.4.7 + # via apache-superset +certifi==2020.6.20 + # via requests +cffi==1.14.3 + # via cryptography +cfgv==3.2.0 + # via pre-commit +chardet==3.0.4 + # via + # aiohttp + # requests + # tabulator +click==7.1.2 + # via + # apache-superset + # flask + # flask-appbuilder + # pip-compile-multi + # pip-tools + # tableschema + # tabulator +colorama==0.4.4 + # via + # apache-superset + # flask-appbuilder +contextlib2==0.6.0.post1 + # via apache-superset +convertdate==2.3.0 + # via holidays coverage==5.3 # via pytest-cov +cron-descriptor==1.2.24 + # via apache-superset +croniter==0.3.36 + # via apache-superset +cryptography==3.3.2 + # via apache-superset +decorator==4.4.2 + # via + # ipython + # retry +defusedxml==0.6.0 + # via python3-openid +deprecated==1.2.11 + # via pygithub +deprecation==2.1.0 + # via apache-superset +distlib==0.3.1 + # via virtualenv +dnspython==2.0.0 + # via email-validator docker==4.3.1 # via -r requirements/testing.in +email-validator==1.1.1 + # via flask-appbuilder +et-xmlfile==1.0.1 + # via openpyxl +filelock==3.0.12 + # via + # tox + # virtualenv +flask==1.1.2 + # via + # apache-superset + # flask-appbuilder + # flask-babel + # flask-caching + # flask-compress + # flask-cors + # flask-jwt-extended + # flask-login + # flask-migrate + # flask-openid + # flask-sqlalchemy + # flask-testing + # flask-wtf +flask-appbuilder==3.3.0 + # via apache-superset +flask-babel==1.0.0 + # via flask-appbuilder +flask-caching==1.10.1 + # via apache-superset +flask-compress==1.8.0 + # via apache-superset +flask-cors==3.0.9 + # via -r requirements/development.in +flask-jwt-extended==3.24.1 + # via flask-appbuilder +flask-login==0.4.1 + # via flask-appbuilder +flask-migrate==2.5.3 + # via apache-superset +flask-openid==1.2.5 + # via flask-appbuilder +flask-sqlalchemy==2.4.4 + # via + # flask-appbuilder + # flask-migrate +flask-talisman==0.7.0 + # via apache-superset flask-testing==0.8.0 # via -r requirements/testing.in +flask-wtf==0.14.3 + # via + # apache-superset + # flask-appbuilder freezegun==1.0.0 # via -r requirements/testing.in +future==0.18.2 + # via pyhive +geographiclib==1.50 + # via geopy +geopy==2.0.0 + # via apache-superset +graphlib-backport==1.0.3 + # via apache-superset +gunicorn==20.0.4 + # via apache-superset +holidays==0.10.3 + # via apache-superset +humanize==3.1.0 + # via apache-superset +identify==1.5.9 + # via pre-commit +idna==2.10 + # via + # email-validator + # requests + # yarl +ijson==3.1.2.post0 + # via tabulator iniconfig==1.1.1 # via pytest ipdb==0.13.4 # via -r requirements/testing.in -ipython-genutils==0.2.0 - # via traitlets ipython==7.16.1 # via # -r requirements/testing.in # ipdb +ipython-genutils==0.2.0 + # via traitlets +isodate==0.6.0 + # via + # apache-superset + # tableschema isort==5.6.4 # via pylint +itsdangerous==1.1.0 + # via + # apache-superset + # flask + # flask-wtf +jdcal==1.4.1 + # via openpyxl jedi==0.17.2 # via ipython +jinja2==2.11.3 + # via + # flask + # flask-babel +jmespath==0.10.0 + # via + # boto3 + # botocore +jsonlines==1.2.0 + # via tabulator +jsonschema==3.2.0 + # via + # flask-appbuilder + # openapi-spec-validator + # tableschema +kombu==4.6.11 + # via celery +korean-lunar-calendar==0.2.1 + # via holidays lazy-object-proxy==1.4.3 # via astroid +linear-tsv==1.1.0 + # via tabulator +mako==1.1.3 + # via alembic +markdown==3.3.3 + # via apache-superset +markupsafe==1.1.1 + # via + # jinja2 + # mako + # wtforms +marshmallow==3.9.0 + # via + # flask-appbuilder + # marshmallow-enum + # marshmallow-sqlalchemy +marshmallow-enum==1.5.1 + # via flask-appbuilder +marshmallow-sqlalchemy==0.23.1 + # via flask-appbuilder mccabe==0.6.1 # via pylint +msgpack==1.0.0 + # via apache-superset +multidict==5.0.0 + # via + # aiohttp + # yarl +mysqlclient==1.4.2.post1 + # via -r requirements/development.in +natsort==7.0.1 + # via croniter +nodeenv==1.5.0 + # via pre-commit +numpy==1.19.4 + # via + # pandas + # pyarrow openapi-spec-validator==0.2.9 # via -r requirements/testing.in +openpyxl==3.0.5 + # via + # -r requirements/testing.in + # tabulator +packaging==20.4 + # via + # bleach + # deprecation + # pytest + # tox +pandas==1.2.2 + # via apache-superset parameterized==0.7.4 # via -r requirements/testing.in +parsedatetime==2.6 + # via apache-superset parso==0.7.1 # via jedi +pathlib2==2.3.5 + # via apache-superset pexpect==4.8.0 # via ipython +pgsanity==0.2.9 + # via apache-superset pickleshare==0.7.5 # via ipython +pillow==7.2.0 + # via -r requirements/development.in +pip-compile-multi==2.4.1 + # via -r requirements/integration.in +pip-tools==5.3.1 + # via pip-compile-multi +pluggy==0.13.1 + # via + # pytest + # tox +polyline==1.4.0 + # via apache-superset +pre-commit==2.8.2 + # via -r requirements/integration.in +prison==0.1.3 + # via flask-appbuilder prompt-toolkit==3.0.8 # via ipython +psycopg2-binary==2.8.5 + # via -r requirements/development.in ptyprocess==0.6.0 # via pexpect +py==1.9.0 + # via + # pytest + # retry + # tox +pyarrow==4.0.1 + # via apache-superset +pycparser==2.20 + # via cffi +pydruid==0.6.1 + # via -r requirements/development.in pyfakefs==4.4.0 # via -r requirements/testing.in +pygithub==1.54.1 + # via -r requirements/development.in pygments==2.7.2 # via ipython pyhive[hive,presto]==0.6.3 # via # -r requirements/development.in # -r requirements/testing.in +pyjwt==1.7.1 + # via + # apache-superset + # flask-appbuilder + # flask-jwt-extended + # pygithub pylint==2.6.0 # via -r requirements/testing.in -pytest-cov==2.10.1 - # via -r requirements/testing.in +pymeeus==0.3.7 + # via convertdate +pyparsing==2.4.7 + # via + # apache-superset + # packaging +pyrsistent==0.16.1 + # via + # -r requirements/base.in + # jsonschema pytest==6.1.2 # via # -r requirements/testing.in # pytest-cov + # pytest-mock +pytest-cov==2.10.1 + # via -r requirements/testing.in +pytest-mock==3.6.1 + # via + # -r requirements/development.in + # -r requirements/testing.in +python-dateutil==2.8.1 + # via + # alembic + # apache-superset + # botocore + # croniter + # flask-appbuilder + # freezegun + # holidays + # pandas + # pyhive + # tableschema +python-dotenv==0.15.0 + # via apache-superset +python-editor==1.0.4 + # via alembic +python-geohash==0.8.5 + # via apache-superset +python3-openid==3.2.0 + # via flask-openid +pytz==2020.4 + # via + # babel + # celery + # convertdate + # flask-babel + # pandas +pyyaml==5.4.1 + # via + # apache-superset + # apispec + # openapi-spec-validator + # pre-commit +redis==3.5.3 + # via apache-superset +requests==2.24.0 + # via + # docker + # pydruid + # pygithub + # pyhive + # tableschema + # tabulator +retry==0.9.2 + # via apache-superset +rfc3986==1.4.0 + # via tableschema +s3transfer==0.3.3 + # via boto3 +sasl==0.2.1 + # via + # pyhive + # thrift-sasl +selenium==3.141.0 + # via apache-superset +simplejson==3.17.2 + # via apache-superset +six==1.15.0 + # via + # astroid + # bleach + # cryptography + # docker + # flask-cors + # flask-jwt-extended + # flask-talisman + # holidays + # isodate + # jsonlines + # jsonschema + # linear-tsv + # openapi-spec-validator + # packaging + # pathlib2 + # pip-tools + # polyline + # prison + # pyrsistent + # python-dateutil + # sasl + # sqlalchemy-utils + # tableschema + # tabulator + # thrift + # thrift-sasl + # tox + # virtualenv + # websocket-client + # wtforms-json +slackclient==2.5.0 + # via apache-superset +sqlalchemy==1.3.20 + # via + # alembic + # apache-superset + # flask-appbuilder + # flask-sqlalchemy + # marshmallow-sqlalchemy + # sqlalchemy-utils + # tabulator +sqlalchemy-utils==0.36.8 + # via + # apache-superset + # flask-appbuilder +sqlparse==0.3.0 + # via apache-superset statsd==3.3.0 # via -r requirements/testing.in +tableschema==1.20.0 + # via -r requirements/development.in +tabulator==1.52.5 + # via tableschema +thrift==0.13.0 + # via + # -r requirements/development.in + # pyhive + # thrift-sasl +thrift-sasl==0.4.2 + # via pyhive +toml==0.10.2 + # via + # pre-commit + # pylint + # pytest + # tox +toposort==1.5 + # via pip-compile-multi +tox==3.20.1 + # via -r requirements/integration.in traitlets==5.0.5 # via ipython -typed-ast==1.4.3 - # via astroid +typing-extensions==3.7.4.3 + # via + # aiohttp + # apache-superset +unicodecsv==0.14.1 + # via + # tableschema + # tabulator +urllib3==1.25.11 + # via + # botocore + # requests + # selenium +vine==1.3.0 + # via + # amqp + # celery +virtualenv==20.1.0 + # via + # pre-commit + # tox wcwidth==0.2.5 # via prompt-toolkit +webencodings==0.5.1 + # via bleach websocket-client==0.57.0 # via docker +werkzeug==1.0.1 + # via + # flask + # flask-jwt-extended +wrapt==1.12.1 + # via + # astroid + # deprecated +wtforms==2.3.3 + # via + # flask-wtf + # wtforms-json +wtforms-json==0.3.3 + # via apache-superset +xlrd==1.2.0 + # via tabulator +yarl==1.6.2 + # via aiohttp +zipp==3.4.1 + # via -r requirements/base.in # The following packages are considered to be unsafe in a requirements file: # pip diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py index 0f5f6ef3b1d96..e41738d3653de 100644 --- a/superset/db_engine_specs/gsheets.py +++ b/superset/db_engine_specs/gsheets.py @@ -17,14 +17,17 @@ import json import re from contextlib import closing -from typing import Any, Dict, Optional, Pattern, Tuple, TYPE_CHECKING +from typing import Any, Dict, List, Optional, Pattern, Tuple, TYPE_CHECKING +from flask import g from flask_babel import gettext as __ +from sqlalchemy.engine import create_engine from sqlalchemy.engine.url import URL +from typing_extensions import TypedDict from superset import security_manager from superset.db_engine_specs.sqlite import SqliteEngineSpec -from superset.errors import SupersetErrorType +from superset.errors import ErrorLevel, SupersetError, SupersetErrorType if TYPE_CHECKING: from superset.models.core import Database @@ -33,6 +36,12 @@ SYNTAX_ERROR_REGEX = re.compile('SQLError: near "(?P.*?)": syntax error') +class GSheetsParametersType(TypedDict): + credentials_info: Dict[str, Any] + query: Dict[str, Any] + catalog: Dict[str, str] + + class GSheetsEngineSpec(SqliteEngineSpec): """Engine for Google spreadsheets""" @@ -45,7 +54,7 @@ class GSheetsEngineSpec(SqliteEngineSpec): SYNTAX_ERROR_REGEX: ( __( 'Please check your query for syntax errors near "%(server_error)s". ' - "Then, try running your query again." + "Then, try running your query again.", ), SupersetErrorType.SYNTAX_ERROR, {}, @@ -54,7 +63,7 @@ class GSheetsEngineSpec(SqliteEngineSpec): @classmethod def modify_url_for_impersonation( - cls, url: URL, impersonate_user: bool, username: Optional[str] + cls, url: URL, impersonate_user: bool, username: Optional[str], ) -> None: if impersonate_user and username is not None: user = security_manager.find_user(username=username) @@ -77,3 +86,41 @@ def extra_table_metadata( metadata = {} return {"metadata": metadata["extra"]} + + @classmethod + def validate_parameters( + cls, parameters: GSheetsParametersType, + ) -> List[SupersetError]: + errors: List[SupersetError] = [] + + credentials_info = parameters.get("credentials_info") + catalog = parameters.get("catalog", {}) + + if not catalog: + return errors + + # We need a subject in case domain wide delegation is set, otherwise the + # check will fail. This means that the admin will be able to add sheets + # that only they have access, even if later users are not able to access + # them. + subject = g.user.email if g.user else None + + engine = create_engine( + "gsheets://", service_account_info=credentials_info, subject=subject, + ) + conn = engine.connect() + for name, url in catalog.items(): + try: + results = conn.execute(f'SELECT * FROM "{url}" LIMIT 1') + results.fetchall() + except Exception: # pylint: disable=broad-except + errors.append( + SupersetError( + message=f"Unable to connect to spreadsheet {name} at {url}", + error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR, + level=ErrorLevel.WARNING, + extra={"name": name, "url": url}, + ), + ) + + return errors diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py new file mode 100644 index 0000000000000..63f964f8f5ddb --- /dev/null +++ b/tests/unit_tests/conftest.py @@ -0,0 +1,30 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +import pytest + +from superset.app import create_app + + +@pytest.fixture +def app_context(): + """ + A fixture for running the test inside an app context. + """ + app = create_app() + with app.app_context(): + yield diff --git a/tests/unit_tests/db_engine_specs/test_gsheets.py b/tests/unit_tests/db_engine_specs/test_gsheets.py new file mode 100644 index 0000000000000..bc8ff1a6c5908 --- /dev/null +++ b/tests/unit_tests/db_engine_specs/test_gsheets.py @@ -0,0 +1,177 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# pylint: disable=unused-argument, invalid-name +from superset.errors import ErrorLevel, SupersetError, SupersetErrorType + + +class ProgrammingError(Exception): + """ + Dummy ProgrammingError so we don't need to import the optional gsheets. + """ + + +def test_validate_parameters_simple(mocker, app_context): + from superset.db_engine_specs.gsheets import GSheetsEngineSpec + + parameters = {} + errors = GSheetsEngineSpec.validate_parameters(parameters) + assert errors == [] + + +def test_validate_parameters_catalog(mocker, app_context): + from superset.db_engine_specs.gsheets import GSheetsEngineSpec + + g = mocker.patch("superset.db_engine_specs.gsheets.g") + g.user.email = "admin@example.com" + + create_engine = mocker.patch("superset.db_engine_specs.gsheets.create_engine") + conn = create_engine.return_value.connect.return_value + results = conn.execute.return_value + results.fetchall.side_effect = [ + ProgrammingError("The caller does not have permission"), + [(1,)], + ProgrammingError("Unsupported table: https://www.google.com/"), + ] + + parameters = { + "catalog": { + "private_sheet": "https://docs.google.com/spreadsheets/d/1/edit", + "public_sheet": "https://docs.google.com/spreadsheets/d/1/edit#gid=1", + "not_a_sheet": "https://www.google.com/", + }, + } + errors = GSheetsEngineSpec.validate_parameters(parameters) + assert errors == [ + SupersetError( + message=( + "Unable to connect to spreadsheet private_sheet at " + "https://docs.google.com/spreadsheets/d/1/edit" + ), + error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR, + level=ErrorLevel.WARNING, + extra={ + "name": "private_sheet", + "url": "https://docs.google.com/spreadsheets/d/1/edit", + "issue_codes": [ + { + "code": 1003, + "message": ( + "Issue 1003 - There is a syntax error in the SQL query. " + "Perhaps there was a misspelling or a typo." + ), + }, + { + "code": 1005, + "message": ( + "Issue 1005 - The table was deleted or renamed in the " + "database." + ), + }, + ], + }, + ), + SupersetError( + message=( + "Unable to connect to spreadsheet not_a_sheet at " + "https://www.google.com/" + ), + error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR, + level=ErrorLevel.WARNING, + extra={ + "name": "not_a_sheet", + "url": "https://www.google.com/", + "issue_codes": [ + { + "code": 1003, + "message": ( + "Issue 1003 - There is a syntax error in the SQL query. " + "Perhaps there was a misspelling or a typo." + ), + }, + { + "code": 1005, + "message": ( + "Issue 1005 - The table was deleted or renamed in the " + "database.", + ), + }, + ], + }, + ), + ] + create_engine.assert_called_with( + "gsheets://", service_account_info=None, subject="admin@example.com", + ) + + +def test_validate_parameters_catalog_and_credentials(mocker, app_context): + from superset.db_engine_specs.gsheets import GSheetsEngineSpec + + g = mocker.patch("superset.db_engine_specs.gsheets.g") + g.user.email = "admin@example.com" + + create_engine = mocker.patch("superset.db_engine_specs.gsheets.create_engine") + conn = create_engine.return_value.connect.return_value + results = conn.execute.return_value + results.fetchall.side_effect = [ + [(2,)], + [(1,)], + ProgrammingError("Unsupported table: https://www.google.com/"), + ] + + parameters = { + "catalog": { + "private_sheet": "https://docs.google.com/spreadsheets/d/1/edit", + "public_sheet": "https://docs.google.com/spreadsheets/d/1/edit#gid=1", + "not_a_sheet": "https://www.google.com/", + }, + "credentials_info": "SECRET", + } + errors = GSheetsEngineSpec.validate_parameters(parameters) + assert errors == [ + SupersetError( + message=( + "Unable to connect to spreadsheet not_a_sheet at " + "https://www.google.com/" + ), + error_type=SupersetErrorType.TABLE_DOES_NOT_EXIST_ERROR, + level=ErrorLevel.WARNING, + extra={ + "name": "not_a_sheet", + "url": "https://www.google.com/", + "issue_codes": [ + { + "code": 1003, + "message": ( + "Issue 1003 - There is a syntax error in the SQL query. " + "Perhaps there was a misspelling or a typo." + ), + }, + { + "code": 1005, + "message": ( + "Issue 1005 - The table was deleted or renamed in the " + "database.", + ), + }, + ], + }, + ), + ] + create_engine.assert_called_with( + "gsheets://", service_account_info="SECRET", subject="admin@example.com", + ) From ee217960bc226bfb0720998ba0285fe3c7a4e08f Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 7 Jul 2021 14:39:27 -0700 Subject: [PATCH 2/4] Move import inside fixture --- tests/unit_tests/conftest.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 63f964f8f5ddb..04d793a860e4a 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -17,14 +17,14 @@ import pytest -from superset.app import create_app - @pytest.fixture def app_context(): """ A fixture for running the test inside an app context. """ + from superset.app import create_app + app = create_app() with app.app_context(): yield From 2b4f6eede0009dbc1aeac7726bc3254719d82a8f Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 7 Jul 2021 15:49:30 -0700 Subject: [PATCH 3/4] Update deps --- requirements/development.in | 1 - requirements/development.txt | 12 +- requirements/testing.in | 2 +- requirements/testing.txt | 475 +---------------------------------- 4 files changed, 12 insertions(+), 478 deletions(-) diff --git a/requirements/development.in b/requirements/development.in index 1955141a30124..5ac06dc543399 100644 --- a/requirements/development.in +++ b/requirements/development.in @@ -25,4 +25,3 @@ psycopg2-binary==2.8.5 tableschema thrift>=0.11.0,<1.0.0 pygithub>=1.54.1,<2.0.0 -pytest-mock>=3.5.1,<4 diff --git a/requirements/development.txt b/requirements/development.txt index e8eeeb364cb7d..c065721174d3b 100644 --- a/requirements/development.txt +++ b/requirements/development.txt @@ -1,4 +1,4 @@ -# SHA1:6621b6d160d24719b26c9ccd5049ed48743c456d +# SHA1:1b4d15a41f3498d2eb930ac3d3d4ce5d1f218a2f # # This file is autogenerated by pip-compile-multi # To update, run: @@ -28,8 +28,6 @@ future==0.18.2 # via pyhive ijson==3.1.2.post0 # via tabulator -iniconfig==1.1.1 - # via pytest jdcal==1.4.1 # via openpyxl jmespath==0.10.0 @@ -46,8 +44,6 @@ openpyxl==3.0.5 # via tabulator pillow==7.2.0 # via -r requirements/development.in -pluggy==0.13.1 - # via pytest psycopg2-binary==2.8.5 # via -r requirements/development.in pydruid==0.6.1 @@ -56,10 +52,6 @@ pygithub==1.54.1 # via -r requirements/development.in pyhive[hive]==0.6.3 # via -r requirements/development.in -pytest==6.2.4 - # via pytest-mock -pytest-mock==3.6.1 - # via -r requirements/development.in requests==2.24.0 # via # pydruid @@ -85,8 +77,6 @@ thrift==0.13.0 # thrift-sasl thrift-sasl==0.4.2 # via pyhive -toml==0.10.2 - # via pytest unicodecsv==0.14.1 # via # tableschema diff --git a/requirements/testing.in b/requirements/testing.in index d2f0b7011df31..9c67f790a73e8 100644 --- a/requirements/testing.in +++ b/requirements/testing.in @@ -32,4 +32,4 @@ pylint pytest pytest-cov statsd -pytest-mock>=3.5.1,<4 +pytest-mock diff --git a/requirements/testing.txt b/requirements/testing.txt index ef711bc7a6a18..f9fe540c8cc9e 100644 --- a/requirements/testing.txt +++ b/requirements/testing.txt @@ -1,180 +1,28 @@ +# SHA1:d39180c0eb498d1a7dd73b8428e6ab304b728484 +# +# This file is autogenerated by pip-compile-multi +# To update, run: +# +# pip-compile-multi +# +-r development.txt +-r integration.txt -e file:. # via -r requirements/base.in -aiohttp==3.7.2 - # via slackclient -alembic==1.4.3 - # via flask-migrate -amqp==2.6.1 - # via kombu -apispec[yaml]==3.3.2 - # via flask-appbuilder -appdirs==1.4.4 - # via virtualenv appnope==0.1.2 # via ipython astroid==2.4.2 # via pylint -async-timeout==3.0.1 - # via aiohttp -attrs==20.2.0 - # via - # aiohttp - # jsonschema - # pytest -babel==2.8.0 - # via flask-babel backcall==0.2.0 # via ipython -backoff==1.10.0 - # via apache-superset -billiard==3.6.3.0 - # via celery -bleach==3.3.0 - # via apache-superset -boto3==1.16.10 - # via tabulator -botocore==1.19.10 - # via - # boto3 - # s3transfer -brotli==1.0.9 - # via flask-compress -cached-property==1.5.2 - # via tableschema -cachelib==0.1.1 - # via apache-superset -celery==4.4.7 - # via apache-superset -certifi==2020.6.20 - # via requests -cffi==1.14.3 - # via cryptography -cfgv==3.2.0 - # via pre-commit -chardet==3.0.4 - # via - # aiohttp - # requests - # tabulator -click==7.1.2 - # via - # apache-superset - # flask - # flask-appbuilder - # pip-compile-multi - # pip-tools - # tableschema - # tabulator -colorama==0.4.4 - # via - # apache-superset - # flask-appbuilder -contextlib2==0.6.0.post1 - # via apache-superset -convertdate==2.3.0 - # via holidays coverage==5.3 # via pytest-cov -cron-descriptor==1.2.24 - # via apache-superset -croniter==0.3.36 - # via apache-superset -cryptography==3.3.2 - # via apache-superset -decorator==4.4.2 - # via - # ipython - # retry -defusedxml==0.6.0 - # via python3-openid -deprecated==1.2.11 - # via pygithub -deprecation==2.1.0 - # via apache-superset -distlib==0.3.1 - # via virtualenv -dnspython==2.0.0 - # via email-validator docker==4.3.1 # via -r requirements/testing.in -email-validator==1.1.1 - # via flask-appbuilder -et-xmlfile==1.0.1 - # via openpyxl -filelock==3.0.12 - # via - # tox - # virtualenv -flask==1.1.2 - # via - # apache-superset - # flask-appbuilder - # flask-babel - # flask-caching - # flask-compress - # flask-cors - # flask-jwt-extended - # flask-login - # flask-migrate - # flask-openid - # flask-sqlalchemy - # flask-testing - # flask-wtf -flask-appbuilder==3.3.0 - # via apache-superset -flask-babel==1.0.0 - # via flask-appbuilder -flask-caching==1.10.1 - # via apache-superset -flask-compress==1.8.0 - # via apache-superset -flask-cors==3.0.9 - # via -r requirements/development.in -flask-jwt-extended==3.24.1 - # via flask-appbuilder -flask-login==0.4.1 - # via flask-appbuilder -flask-migrate==2.5.3 - # via apache-superset -flask-openid==1.2.5 - # via flask-appbuilder -flask-sqlalchemy==2.4.4 - # via - # flask-appbuilder - # flask-migrate -flask-talisman==0.7.0 - # via apache-superset flask-testing==0.8.0 # via -r requirements/testing.in -flask-wtf==0.14.3 - # via - # apache-superset - # flask-appbuilder freezegun==1.0.0 # via -r requirements/testing.in -future==0.18.2 - # via pyhive -geographiclib==1.50 - # via geopy -geopy==2.0.0 - # via apache-superset -graphlib-backport==1.0.3 - # via apache-superset -gunicorn==20.0.4 - # via apache-superset -holidays==0.10.3 - # via apache-superset -humanize==3.1.0 - # via apache-superset -identify==1.5.9 - # via pre-commit -idna==2.10 - # via - # email-validator - # requests - # yarl -ijson==3.1.2.post0 - # via tabulator iniconfig==1.1.1 # via pytest ipdb==0.13.4 @@ -185,169 +33,38 @@ ipython==7.16.1 # ipdb ipython-genutils==0.2.0 # via traitlets -isodate==0.6.0 - # via - # apache-superset - # tableschema isort==5.6.4 # via pylint -itsdangerous==1.1.0 - # via - # apache-superset - # flask - # flask-wtf -jdcal==1.4.1 - # via openpyxl jedi==0.17.2 # via ipython -jinja2==2.11.3 - # via - # flask - # flask-babel -jmespath==0.10.0 - # via - # boto3 - # botocore -jsonlines==1.2.0 - # via tabulator -jsonschema==3.2.0 - # via - # flask-appbuilder - # openapi-spec-validator - # tableschema -kombu==4.6.11 - # via celery -korean-lunar-calendar==0.2.1 - # via holidays lazy-object-proxy==1.4.3 # via astroid -linear-tsv==1.1.0 - # via tabulator -mako==1.1.3 - # via alembic -markdown==3.3.3 - # via apache-superset -markupsafe==1.1.1 - # via - # jinja2 - # mako - # wtforms -marshmallow==3.9.0 - # via - # flask-appbuilder - # marshmallow-enum - # marshmallow-sqlalchemy -marshmallow-enum==1.5.1 - # via flask-appbuilder -marshmallow-sqlalchemy==0.23.1 - # via flask-appbuilder mccabe==0.6.1 # via pylint -msgpack==1.0.0 - # via apache-superset -multidict==5.0.0 - # via - # aiohttp - # yarl -mysqlclient==1.4.2.post1 - # via -r requirements/development.in -natsort==7.0.1 - # via croniter -nodeenv==1.5.0 - # via pre-commit -numpy==1.19.4 - # via - # pandas - # pyarrow openapi-spec-validator==0.2.9 # via -r requirements/testing.in -openpyxl==3.0.5 - # via - # -r requirements/testing.in - # tabulator -packaging==20.4 - # via - # bleach - # deprecation - # pytest - # tox -pandas==1.2.2 - # via apache-superset parameterized==0.7.4 # via -r requirements/testing.in -parsedatetime==2.6 - # via apache-superset parso==0.7.1 # via jedi -pathlib2==2.3.5 - # via apache-superset pexpect==4.8.0 # via ipython -pgsanity==0.2.9 - # via apache-superset pickleshare==0.7.5 # via ipython -pillow==7.2.0 - # via -r requirements/development.in -pip-compile-multi==2.4.1 - # via -r requirements/integration.in -pip-tools==5.3.1 - # via pip-compile-multi -pluggy==0.13.1 - # via - # pytest - # tox -polyline==1.4.0 - # via apache-superset -pre-commit==2.8.2 - # via -r requirements/integration.in -prison==0.1.3 - # via flask-appbuilder prompt-toolkit==3.0.8 # via ipython -psycopg2-binary==2.8.5 - # via -r requirements/development.in ptyprocess==0.6.0 # via pexpect -py==1.9.0 - # via - # pytest - # retry - # tox -pyarrow==4.0.1 - # via apache-superset -pycparser==2.20 - # via cffi -pydruid==0.6.1 - # via -r requirements/development.in pyfakefs==4.4.0 # via -r requirements/testing.in -pygithub==1.54.1 - # via -r requirements/development.in pygments==2.7.2 # via ipython pyhive[hive,presto]==0.6.3 # via # -r requirements/development.in # -r requirements/testing.in -pyjwt==1.7.1 - # via - # apache-superset - # flask-appbuilder - # flask-jwt-extended - # pygithub pylint==2.6.0 # via -r requirements/testing.in -pymeeus==0.3.7 - # via convertdate -pyparsing==2.4.7 - # via - # apache-superset - # packaging -pyrsistent==0.16.1 - # via - # -r requirements/base.in - # jsonschema pytest==6.1.2 # via # -r requirements/testing.in @@ -356,187 +73,15 @@ pytest==6.1.2 pytest-cov==2.10.1 # via -r requirements/testing.in pytest-mock==3.6.1 - # via - # -r requirements/development.in - # -r requirements/testing.in -python-dateutil==2.8.1 - # via - # alembic - # apache-superset - # botocore - # croniter - # flask-appbuilder - # freezegun - # holidays - # pandas - # pyhive - # tableschema -python-dotenv==0.15.0 - # via apache-superset -python-editor==1.0.4 - # via alembic -python-geohash==0.8.5 - # via apache-superset -python3-openid==3.2.0 - # via flask-openid -pytz==2020.4 - # via - # babel - # celery - # convertdate - # flask-babel - # pandas -pyyaml==5.4.1 - # via - # apache-superset - # apispec - # openapi-spec-validator - # pre-commit -redis==3.5.3 - # via apache-superset -requests==2.24.0 - # via - # docker - # pydruid - # pygithub - # pyhive - # tableschema - # tabulator -retry==0.9.2 - # via apache-superset -rfc3986==1.4.0 - # via tableschema -s3transfer==0.3.3 - # via boto3 -sasl==0.2.1 - # via - # pyhive - # thrift-sasl -selenium==3.141.0 - # via apache-superset -simplejson==3.17.2 - # via apache-superset -six==1.15.0 - # via - # astroid - # bleach - # cryptography - # docker - # flask-cors - # flask-jwt-extended - # flask-talisman - # holidays - # isodate - # jsonlines - # jsonschema - # linear-tsv - # openapi-spec-validator - # packaging - # pathlib2 - # pip-tools - # polyline - # prison - # pyrsistent - # python-dateutil - # sasl - # sqlalchemy-utils - # tableschema - # tabulator - # thrift - # thrift-sasl - # tox - # virtualenv - # websocket-client - # wtforms-json -slackclient==2.5.0 - # via apache-superset -sqlalchemy==1.3.20 - # via - # alembic - # apache-superset - # flask-appbuilder - # flask-sqlalchemy - # marshmallow-sqlalchemy - # sqlalchemy-utils - # tabulator -sqlalchemy-utils==0.36.8 - # via - # apache-superset - # flask-appbuilder -sqlparse==0.3.0 - # via apache-superset + # via -r requirements/testing.in statsd==3.3.0 # via -r requirements/testing.in -tableschema==1.20.0 - # via -r requirements/development.in -tabulator==1.52.5 - # via tableschema -thrift==0.13.0 - # via - # -r requirements/development.in - # pyhive - # thrift-sasl -thrift-sasl==0.4.2 - # via pyhive -toml==0.10.2 - # via - # pre-commit - # pylint - # pytest - # tox -toposort==1.5 - # via pip-compile-multi -tox==3.20.1 - # via -r requirements/integration.in traitlets==5.0.5 # via ipython -typing-extensions==3.7.4.3 - # via - # aiohttp - # apache-superset -unicodecsv==0.14.1 - # via - # tableschema - # tabulator -urllib3==1.25.11 - # via - # botocore - # requests - # selenium -vine==1.3.0 - # via - # amqp - # celery -virtualenv==20.1.0 - # via - # pre-commit - # tox wcwidth==0.2.5 # via prompt-toolkit -webencodings==0.5.1 - # via bleach websocket-client==0.57.0 # via docker -werkzeug==1.0.1 - # via - # flask - # flask-jwt-extended -wrapt==1.12.1 - # via - # astroid - # deprecated -wtforms==2.3.3 - # via - # flask-wtf - # wtforms-json -wtforms-json==0.3.3 - # via apache-superset -xlrd==1.2.0 - # via tabulator -yarl==1.6.2 - # via aiohttp -zipp==3.4.1 - # via -r requirements/base.in # The following packages are considered to be unsafe in a requirements file: # pip From 17c7ab534fef5ab3951086a5f8689909cc1e4a49 Mon Sep 17 00:00:00 2001 From: Beto Dealmeida Date: Wed, 7 Jul 2021 16:24:03 -0700 Subject: [PATCH 4/4] Rename parameter --- superset/db_engine_specs/gsheets.py | 8 ++++---- tests/unit_tests/db_engine_specs/test_gsheets.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/superset/db_engine_specs/gsheets.py b/superset/db_engine_specs/gsheets.py index e41738d3653de..0becf050647e2 100644 --- a/superset/db_engine_specs/gsheets.py +++ b/superset/db_engine_specs/gsheets.py @@ -39,7 +39,7 @@ class GSheetsParametersType(TypedDict): credentials_info: Dict[str, Any] query: Dict[str, Any] - catalog: Dict[str, str] + table_catalog: Dict[str, str] class GSheetsEngineSpec(SqliteEngineSpec): @@ -94,9 +94,9 @@ def validate_parameters( errors: List[SupersetError] = [] credentials_info = parameters.get("credentials_info") - catalog = parameters.get("catalog", {}) + table_catalog = parameters.get("table_catalog", {}) - if not catalog: + if not table_catalog: return errors # We need a subject in case domain wide delegation is set, otherwise the @@ -109,7 +109,7 @@ def validate_parameters( "gsheets://", service_account_info=credentials_info, subject=subject, ) conn = engine.connect() - for name, url in catalog.items(): + for name, url in table_catalog.items(): try: results = conn.execute(f'SELECT * FROM "{url}" LIMIT 1') results.fetchall() diff --git a/tests/unit_tests/db_engine_specs/test_gsheets.py b/tests/unit_tests/db_engine_specs/test_gsheets.py index bc8ff1a6c5908..1ced5969726bf 100644 --- a/tests/unit_tests/db_engine_specs/test_gsheets.py +++ b/tests/unit_tests/db_engine_specs/test_gsheets.py @@ -48,7 +48,7 @@ def test_validate_parameters_catalog(mocker, app_context): ] parameters = { - "catalog": { + "table_catalog": { "private_sheet": "https://docs.google.com/spreadsheets/d/1/edit", "public_sheet": "https://docs.google.com/spreadsheets/d/1/edit#gid=1", "not_a_sheet": "https://www.google.com/", @@ -134,7 +134,7 @@ def test_validate_parameters_catalog_and_credentials(mocker, app_context): ] parameters = { - "catalog": { + "table_catalog": { "private_sheet": "https://docs.google.com/spreadsheets/d/1/edit", "public_sheet": "https://docs.google.com/spreadsheets/d/1/edit#gid=1", "not_a_sheet": "https://www.google.com/",