Skip to content

Commit

Permalink
SNOW-716364: Revert client app id and add connector regression tests (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
sfc-gh-aling authored Jan 10, 2023
1 parent a84fa77 commit 9596d0a
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 24 deletions.
43 changes: 42 additions & 1 deletion .github/workflows/build_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[submodule "tests/connector_regression"]
path = tests/connector_regression
url = [email protected]:snowflakedb/snowflake-connector-python
7 changes: 0 additions & 7 deletions src/snowflake/sqlalchemy/snowdialect.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
_CUSTOM_Float,
_CUSTOM_Time,
)
from .util import _update_connection_application_name

colspecs = {
Date: _CUSTOM_Date,
Expand Down Expand Up @@ -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):
Expand Down
1 change: 1 addition & 0 deletions tests/connector_regression
Submodule connector_regression added at a31324
41 changes: 27 additions & 14 deletions tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -135,7 +148,7 @@ def test_connect_args():
)
)
try:
verify_engine_connection(engine)
verify_engine_connection(engine, verify_app_name)
finally:
engine.dispose()

Expand All @@ -150,7 +163,7 @@ def test_connect_args():
)
)
try:
verify_engine_connection(engine)
verify_engine_connection(engine, verify_app_name)
finally:
engine.dispose()

Expand All @@ -166,7 +179,7 @@ def test_connect_args():
)
)
try:
verify_engine_connection(engine)
verify_engine_connection(engine, verify_app_name)
finally:
engine.dispose()

Expand Down
20 changes: 18 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 9596d0a

Please sign in to comment.