SNOW-1058245-sqlalchemy-20-support: after rebase fixes

snowflakedb · Mar 26, 2024 · 5bfa71a · 5bfa71a
1 parent 0660cf9
commit 5bfa71a
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 73 deletions.
diff --git a/.github/workflows/build_test.yml b/.github/workflows/build_test.yml
@@ -137,8 +137,8 @@ jobs:
           path: |
             ./coverage.xml
 
-  test-dialect-run-v20:
-    name: Test dialect run v20 ${{ matrix.os }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
+  test-dialect-v20:
+    name: Test dialect v20 ${{ matrix.os }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
     needs: lint
     runs-on: ${{ matrix.os }}
     strategy:
@@ -176,10 +176,56 @@ jobs:
           python -m pip install -U hatch
           python -m hatch env create default
       - name: Run tests
-        run: hatch run test-run_v20
+        run: hatch run sa20:test-dialect
       - uses: actions/upload-artifact@v4
         with:
-          name: coverage.xml_dialect-run-20-${{ matrix.os }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
+          name: coverage.xml_dialect-v20-${{ matrix.os }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
+          path: |
+            ./coverage.xml
+
+  test-dialect-compatibility-v20:
+    name: Test dialect v20 compatibility ${{ matrix.os }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
+    needs: lint
+    runs-on: ${{ matrix.os }}
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [
+          ubuntu-latest,
+          macos-latest,
+          windows-latest,
+          ]
+        python-version: ["3.8"]
+        cloud-provider: [
+          aws,
+          azure,
+          gcp,
+          ]
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          persist-credentials: false
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: Upgrade pip and install hatch
+        run: |
+          python -m pip install -U pip
+          python -m pip install -U hatch
+          python -m hatch env create default
+      - 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/parameters.py
+      - name: Run tests
+        run: hatch run sa20:test-dialect-compatibility
+      - uses: actions/upload-artifact@v4
+        with:
+          name: coverage.xml_dialect-v20-compatibility-${{ matrix.os }}-${{ matrix.python-version }}-${{ matrix.cloud-provider }}
           path: |
             ./coverage.xml
 

diff --git a/tests/conftest.py b/tests/conftest.py
@@ -46,16 +46,6 @@
 TEST_SCHEMA = f"sqlalchemy_tests_{str(uuid.uuid4()).replace('-', '_')}"
 
 
-def pytest_addoption(parser):
-    parser.addoption(
-        "--run_v20_sqlalchemy",
-        help="Use only 2.0 SQLAlchemy APIs, any legacy features (< 2.0) will not be supported."
-        "Turning on this option will set future flag to True on Engine and Session objects according to"
-        "the migration guide: https://docs.sqlalchemy.org/en/14/changelog/migration_20.html",
-        action="store_true",
-    )
-
-
 @pytest.fixture(scope="session")
 def on_travis():
     return os.getenv("TRAVIS", "").lower() == "true"
@@ -155,38 +145,42 @@ def url_factory(**kwargs) -> URL:
     return URL(**url_params)
 
 
-def get_engine(url: URL, run_v20_sqlalchemy=False, **engine_kwargs):
+def get_engine(url: URL, **engine_kwargs):
     engine_params = {
         "poolclass": NullPool,
-        "future": run_v20_sqlalchemy,
+        "future": True,
+        "echo": True,
     }
     engine_params.update(engine_kwargs)
-    engine = create_engine(url, **engine_kwargs)
+    engine = create_engine(url, **engine_params)
     return engine
 
 
+# @pytest.fixture(scope="session")
 @pytest.fixture()
-def engine_testaccount(request, run_v20_sqlalchemy):
+def engine_testaccount(request):
     url = url_factory()
-    engine = get_engine(url, run_v20_sqlalchemy=run_v20_sqlalchemy)
+    engine = get_engine(url)
     request.addfinalizer(engine.dispose)
     yield engine
 
 
+# @pytest.fixture(scope="session")
 @pytest.fixture()
 def engine_testaccount_with_numpy(request):
     url = url_factory(numpy=True)
-    engine = get_engine(url, run_v20_sqlalchemy=run_v20_sqlalchemy)
+    engine = get_engine(url)
     request.addfinalizer(engine.dispose)
     yield engine
 
 
+# @pytest.fixture(scope="session")
 @pytest.fixture()
-def engine_testaccount_with_qmark(request, run_v20_sqlalchemy):
+def engine_testaccount_with_qmark(request):
     snowflake.connector.paramstyle = "qmark"
 
     url = url_factory()
-    engine = get_engine(url, run_v20_sqlalchemy=run_v20_sqlalchemy)
+    engine = get_engine(url)
     request.addfinalizer(engine.dispose)
 
     yield engine

diff --git a/tests/test_orm.py b/tests/test_orm.py
@@ -20,7 +20,7 @@
 from sqlalchemy.orm import Session, declarative_base, relationship
 
 
-def test_basic_orm(engine_testaccount, run_v20_sqlalchemy):
+def test_basic_orm(engine_testaccount):
     """
     Tests declarative
     """
@@ -46,7 +46,6 @@ def __repr__(self):
         ed_user = User(name="ed", fullname="Edward Jones")
 
         session = Session(bind=engine_testaccount)
-        session.future = run_v20_sqlalchemy
         session.add(ed_user)
 
         our_user = session.query(User).filter_by(name="ed").first()
@@ -56,7 +55,7 @@ def __repr__(self):
         Base.metadata.drop_all(engine_testaccount)
 
 
-def test_orm_one_to_many_relationship(engine_testaccount, run_v20_sqlalchemy):
+def test_orm_one_to_many_relationship(engine_testaccount):
     """
     Tests One to Many relationship
     """
@@ -97,7 +96,6 @@ def __repr__(self):
         ]
 
         session = Session(bind=engine_testaccount)
-        session.future = run_v20_sqlalchemy
         session.add(jack)  # cascade each Address into the Session as well
         session.commit()
 
@@ -124,7 +122,7 @@ def __repr__(self):
         Base.metadata.drop_all(engine_testaccount)
 
 
-def test_delete_cascade(engine_testaccount, run_v20_sqlalchemy):
+def test_delete_cascade(engine_testaccount):
     """
     Test delete cascade
     """
@@ -169,7 +167,6 @@ def __repr__(self):
         ]
 
         session = Session(bind=engine_testaccount)
-        session.future = run_v20_sqlalchemy
         session.add(jack)  # cascade each Address into the Session as well
         session.commit()
 
@@ -189,7 +186,7 @@ def __repr__(self):
 WIP
 """,
 )
-def test_orm_query(engine_testaccount, run_v20_sqlalchemy):
+def test_orm_query(engine_testaccount):
     """
     Tests ORM query
     """
@@ -210,7 +207,6 @@ def __repr__(self):
     # TODO: insert rows
 
     session = Session(bind=engine_testaccount)
-    session.future = run_v20_sqlalchemy
 
     # TODO: query.all()
     for name, fullname in session.query(User.name, User.fullname):
@@ -220,7 +216,7 @@ def __repr__(self):
         # MultipleResultsFound if not one result
 
 
-def test_schema_including_db(engine_testaccount, db_parameters, run_v20_sqlalchemy):
+def test_schema_including_db(engine_testaccount, db_parameters):
     """
     Test schema parameter including database separated by a dot.
     """
@@ -243,7 +239,6 @@ class User(Base):
         ed_user = User(name="ed", fullname="Edward Jones")
 
         session = Session(bind=engine_testaccount)
-        session.future = run_v20_sqlalchemy
         session.add(ed_user)
 
         ret_user = session.query(User.id, User.name).first()
@@ -255,7 +250,7 @@ class User(Base):
         Base.metadata.drop_all(engine_testaccount)
 
 
-def test_schema_including_dot(engine_testaccount, db_parameters, run_v20_sqlalchemy):
+def test_schema_including_dot(engine_testaccount, db_parameters):
     """
     Tests pseudo schema name including dot.
     """
@@ -276,7 +271,6 @@ class User(Base):
         fullname = Column(String)
 
     session = Session(bind=engine_testaccount)
-    session.future = run_v20_sqlalchemy
     query = session.query(User.id)
     assert str(query).startswith(
         'SELECT {db}."{schema}.{schema}".{db}.users.id'.format(
@@ -285,9 +279,7 @@ class User(Base):
     )
 
 
-def test_schema_translate_map(
-    engine_testaccount, db_parameters, sql_compiler, run_v20_sqlalchemy
-):
+def test_schema_translate_map(engine_testaccount, db_parameters):
     """
     Test schema translate map execution option works replaces schema correctly
     """
@@ -310,7 +302,6 @@ class User(Base):
         schema_translate_map={schema_map: db_parameters["schema"]}
     ) as con:
         session = Session(bind=con)
-        session.future = run_v20_sqlalchemy
         with con.begin():
             Base.metadata.create_all(con)
             try:

diff --git a/tests/test_pandas.py b/tests/test_pandas.py
@@ -27,6 +27,7 @@
 
 from snowflake.connector import ProgrammingError
 from snowflake.connector.pandas_tools import make_pd_writer, pd_writer
+from snowflake.sqlalchemy.compat import IS_VERSION_20
 
 
 def _create_users_addresses_tables(engine_testaccount, metadata):
@@ -240,8 +241,8 @@ def test_timezone(db_parameters, engine_testaccount, engine_testaccount_with_num
             conn.exec_driver_sql(f"DROP TABLE {test_table_name};")
 
 
-def test_pandas_writeback(engine_testaccount, run_v20_sqlalchemy):
-    if run_v20_sqlalchemy and sys.version_info < (3, 8):
+def test_pandas_writeback(engine_testaccount):
+    if IS_VERSION_20 and sys.version_info < (3, 8):
         pytest.skip(
             "In Python 3.7, this test depends on pandas features of which the implementation is incompatible with sqlachemy 2.0, and pandas does not support Python 3.7 anymore."
         )
@@ -352,8 +353,8 @@ def test_pandas_invalid_make_pd_writer(engine_testaccount):
         )
 
 
-def test_percent_signs(engine_testaccount, run_v20_sqlalchemy):
-    if run_v20_sqlalchemy and sys.version_info < (3, 8):
+def test_percent_signs(engine_testaccount):
+    if IS_VERSION_20 and sys.version_info < (3, 8):
         pytest.skip(
             "In Python 3.7, this test depends on pandas features of which the implementation is incompatible with sqlachemy 2.0, and pandas does not support Python 3.7 anymore."
         )
@@ -376,7 +377,7 @@ def test_percent_signs(engine_testaccount, run_v20_sqlalchemy):
             not_like_sql = f"select * from {table_name} where c2 not like '%b%'"
             like_sql = f"select * from {table_name} where c2 like '%b%'"
             calculate_sql = "SELECT 1600 % 400 AS a, 1599 % 400 as b"
-            if run_v20_sqlalchemy:
+            if IS_VERSION_20:
                 not_like_sql = sqlalchemy.text(not_like_sql)
                 like_sql = sqlalchemy.text(like_sql)
                 calculate_sql = sqlalchemy.text(calculate_sql)

diff --git a/tests/test_qmark.py b/tests/test_qmark.py
@@ -12,11 +12,11 @@
 THIS_DIR = os.path.dirname(os.path.realpath(__file__))
 
 
-def test_qmark_bulk_insert(run_v20_sqlalchemy, engine_testaccount_with_qmark):
+def test_qmark_bulk_insert(engine_testaccount_with_qmark):
     """
     Bulk insert using qmark paramstyle
     """
-    if run_v20_sqlalchemy and sys.version_info < (3, 8):
+    if sys.version_info < (3, 8):
         pytest.skip(
             "In Python 3.7, this test depends on pandas features of which the implementation is incompatible with sqlachemy 2.0, and pandas does not support Python 3.7 anymore."
         )

diff --git a/tox.ini b/tox.ini
@@ -2,7 +2,6 @@
 min_version = 4.0.0
 envlist = fix_lint,
           py{37,38,39,310,311}{,-pandas},
-          py{38}-sqlalchemy-v2,
           coverage,
 skip_missing_interpreters = true
 
@@ -45,12 +44,6 @@ commands = pytest \
            --cov "snowflake.sqlalchemy" --cov-append \
            --junitxml {toxworkdir}/junit_{envname}.xml \
            {posargs:tests/sqlalchemy_test_suite}
-           pytest \
-           {env:SNOWFLAKE_PYTEST_OPTS:} \
-           --cov "snowflake.sqlalchemy" --cov-append \
-           --junitxml {toxworkdir}/junit_{envname}.xml \
-           --run_v20_sqlalchemy \
-           {posargs:tests}
 
 [testenv:.pkg_external]
 deps = build
@@ -86,27 +79,6 @@ skip_install = True
 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"))'
 
-[testenv:py{38}-sqlalchemy-v2]
-base_python = py38
-commands =
-  pip install -U sqlalchemy
-  pip list
-  pytest \
-    {env:SNOWFLAKE_PYTEST_OPTS:} \
-    --cov "snowflake.sqlalchemy" \
-    --junitxml {toxworkdir}/junit_{envname}.xml \
-    {posargs:tests}
-    pytest {env:SNOWFLAKE_PYTEST_OPTS:} \
-    --cov "snowflake.sqlalchemy" --cov-append \
-    --junitxml {toxworkdir}/junit_{envname}.xml \
-    {posargs:tests/sqlalchemy_test_suite}
-    pytest \
-    {env:SNOWFLAKE_PYTEST_OPTS:} \
-    --cov "snowflake.sqlalchemy" --cov-append \
-    --junitxml {toxworkdir}/junit_{envname}.xml \
-    --run_v20_sqlalchemy \
-    {posargs:tests}
-
 [pytest]
 addopts = -ra --ignore=tests/sqlalchemy_test_suite
 junit_family = legacy