From 9596d0afaafd3152f505060b813797e5f7dc6a5f Mon Sep 17 00:00:00 2001 From: Adam Ling Date: Tue, 10 Jan 2023 12:28:38 -0800 Subject: [PATCH] SNOW-716364: Revert client app id and add connector regression tests (#369) --- .github/workflows/build_test.yml | 43 ++++++++++++++++++++++++- .gitmodules | 3 ++ src/snowflake/sqlalchemy/snowdialect.py | 7 ---- tests/connector_regression | 1 + tests/test_core.py | 41 +++++++++++++++-------- tox.ini | 20 ++++++++++-- 6 files changed, 91 insertions(+), 24 deletions(-) create mode 100644 .gitmodules create mode 160000 tests/connector_regression diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml index f2a5f873..35c306fa 100644 --- a/.github/workflows/build_test.yml +++ b/.github/workflows/build_test.yml @@ -97,10 +97,51 @@ jobs: .tox/.coverage .tox/coverage.xml + test_connector_regression: + name: Connector Regression Test ${{ matrix.os.download_name }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }} + needs: lint + runs-on: ${{ matrix.os.image_name }} + strategy: + fail-fast: false + matrix: + os: + - image_name: ubuntu-latest + download_name: manylinux_x86_64 + python-version: ["3.7"] + cloud-provider: [aws] + steps: + - uses: actions/checkout@v2 + with: + submodules: true + - name: Set up Python + uses: actions/setup-python@v2 + with: + python-version: ${{ matrix.python-version }} + - name: Display Python version + run: python -c "import sys; print(sys.version)" + - name: Setup parameters file + shell: bash + env: + PARAMETERS_SECRET: ${{ secrets.PARAMETERS_SECRET }} + run: | + gpg --quiet --batch --yes --decrypt --passphrase="$PARAMETERS_SECRET" \ + .github/workflows/parameters/parameters_${{ matrix.cloud-provider }}.py.gpg > tests/connector_regression/test/parameters.py + - name: Upgrade setuptools, pip and wheel + run: python -m pip install -U setuptools pip wheel + - name: Install tox + run: python -m pip install tox + - name: List installed packages + run: python -m pip freeze + - name: Run tests + run: python -m tox -e connector_regression --skip-missing-interpreters false + env: + PYTEST_ADDOPTS: -vvv --color=yes --tb=short + TOX_PARALLEL_NO_SPINNER: 1 + combine-coverage: if: ${{ success() || failure() }} name: Combine coverage - needs: test + needs: [test, test_connector_regression] runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 00000000..6f8702e6 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "tests/connector_regression"] + path = tests/connector_regression + url = git@github.com:snowflakedb/snowflake-connector-python diff --git a/src/snowflake/sqlalchemy/snowdialect.py b/src/snowflake/sqlalchemy/snowdialect.py index 8dde8413..5003f286 100644 --- a/src/snowflake/sqlalchemy/snowdialect.py +++ b/src/snowflake/sqlalchemy/snowdialect.py @@ -61,7 +61,6 @@ _CUSTOM_Float, _CUSTOM_Time, ) -from .util import _update_connection_application_name colspecs = { Date: _CUSTOM_Date, @@ -835,12 +834,6 @@ def get_table_comment(self, connection, table_name, schema=None, **kw): else None } - def connect(self, *cargs, **cparams): - snowflake_conn = super().connect( - *cargs, **_update_connection_application_name(**cparams) - ) - return snowflake_conn - @sa_vnt.listens_for(Table, "before_create") def check_table(table, connection, _ddl_runner, **kw): diff --git a/tests/connector_regression b/tests/connector_regression new file mode 160000 index 00000000..a3132409 --- /dev/null +++ b/tests/connector_regression @@ -0,0 +1 @@ +Subproject commit a313240982f8182ddfa11d031b335ad7d3db3c6f diff --git a/tests/test_core.py b/tests/test_core.py index 46b4eb8f..68a2a12e 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -99,22 +99,35 @@ def _create_users_addresses_tables_without_sequence(engine_testaccount, metadata return users, addresses -def verify_engine_connection(engine): +def verify_engine_connection(engine, verify_app_name): with engine.connect() as conn: results = conn.execute(text("select current_version()")).fetchone() - assert conn.connection.driver_connection.application == APPLICATION_NAME - assert ( - conn.connection.driver_connection._internal_application_name - == APPLICATION_NAME - ) - assert ( - conn.connection.driver_connection._internal_application_version - == SNOWFLAKE_SQLALCHEMY_VERSION - ) + if verify_app_name: + assert conn.connection.driver_connection.application == APPLICATION_NAME + assert ( + conn.connection.driver_connection._internal_application_name + == APPLICATION_NAME + ) + assert ( + conn.connection.driver_connection._internal_application_version + == SNOWFLAKE_SQLALCHEMY_VERSION + ) assert results is not None -def test_connect_args(): +@pytest.mark.parametrize( + "verify_app_name", + [ + False, + pytest.param( + True, + marks=pytest.mark.xfail( + reason="Pending backend service to recognize SnowflakeSQLAlchemy as a valid client app id" + ), + ), + ], +) +def test_connect_args(verify_app_name): """ Tests connect string @@ -135,7 +148,7 @@ def test_connect_args(): ) ) try: - verify_engine_connection(engine) + verify_engine_connection(engine, verify_app_name) finally: engine.dispose() @@ -150,7 +163,7 @@ def test_connect_args(): ) ) try: - verify_engine_connection(engine) + verify_engine_connection(engine, verify_app_name) finally: engine.dispose() @@ -166,7 +179,7 @@ def test_connect_args(): ) ) try: - verify_engine_connection(engine) + verify_engine_connection(engine, verify_app_name) finally: engine.dispose() diff --git a/tox.ini b/tox.ini index 0fcf57e0..69f440b3 100644 --- a/tox.ini +++ b/tox.ini @@ -2,7 +2,8 @@ min_version = 4.0.0 envlist = fix_lint, py{37,38,39,310}{,-pandas}, - coverage + coverage, + connector_regression skip_missing_interpreters = true [testenv] @@ -50,6 +51,13 @@ commands = pytest \ --run_v20_sqlalchemy \ {posargs:tests} +[testenv:connector_regression] +deps = pendulum +commands = pytest \ + {env:SNOWFLAKE_PYTEST_OPTS:} \ + -m "not gcp and not azure" \ + {posargs:tests/connector_regression/test} + [testenv:.pkg_external] deps = build package_glob = {toxinidir}{/}dist{/}*.whl @@ -85,14 +93,22 @@ commands = pre-commit run --all-files python -c 'import pathlib; print("hint: run \{\} install to add checks as pre-commit hook".format(pathlib.Path(r"{envdir}") / "bin" / "pre-commit"))' [pytest] -addopts = -ra --strict-markers --ignore=tests/sqlalchemy_test_suite +addopts = -ra --strict-markers --ignore=tests/sqlalchemy_test_suite --ignore=tests/connector_regression junit_family = legacy log_level = info markers = + # Optional dependency groups markers + lambda: AWS lambda tests + pandas: tests for pandas integration + sso: tests for sso optional dependency integration # Cloud provider markers aws: tests for Amazon Cloud storage azure: tests for Azure Cloud storage gcp: tests for Google Cloud storage + # Test type markers + integ: integration tests + unit: unit tests + skipolddriver: skip for old driver tests # Other markers timeout: tests that need a timeout time internal: tests that could but should only run on our internal CI