From 32d5904b2bbbd6586d4af8b1a704132162cf0195 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Thu, 22 Dec 2022 03:18:58 +0100 Subject: [PATCH 1/2] CI: Fix installation of defined SQLAlchemy version, bringing back SA13 The order of operations was wrong. So, previously, the process would always install SQLAlchemy 1.4, skipping testing on SQLAlchemy 1.3. --- .github/workflows/tests.yml | 3 +++ bootstrap.sh | 9 +++++++++ devtools/setup_ci.sh | 14 -------------- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 4f97c5c1..f514c4fb 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -47,6 +47,9 @@ jobs: # Bootstrap environment. source bootstrap.sh + # Report about the test matrix slot. + echo "Invoking tests with CrateDB ${CRATEDB_VERSION} and SQLAlchemy ${SQLALCHEMY_VERSION}" + # Invoke validation tasks. flake8 src bin coverage run bin/test -vv1 diff --git a/bootstrap.sh b/bootstrap.sh index f78b283c..60d05f4f 100644 --- a/bootstrap.sh +++ b/bootstrap.sh @@ -65,6 +65,15 @@ function setup_package() { # Install package in editable mode. pip install --editable='.[sqlalchemy,test,doc]' + # Install designated SQLAlchemy version. + if [ -n "${SQLALCHEMY_VERSION}" ]; then + if [ "${SQLALCHEMY_VERSION}" = "latest" ]; then + pip install "sqlalchemy" --upgrade + else + pip install "sqlalchemy==${SQLALCHEMY_VERSION}" + fi + fi + } function run_buildout() { diff --git a/devtools/setup_ci.sh b/devtools/setup_ci.sh index 5a5a48ff..b52afe9e 100755 --- a/devtools/setup_ci.sh +++ b/devtools/setup_ci.sh @@ -10,20 +10,6 @@ function main() { echo "Environment variable 'CRATEDB_VERSION' needed" exit 1 } - [ -z ${SQLALCHEMY_VERSION} ] && { - echo "Environment variable 'SQLALCHEMY_VERSION' needed" - exit 1 - } - - # Let's go. - echo "Invoking tests with CrateDB ${CRATEDB_VERSION} and SQLAlchemy ${SQLALCHEMY_VERSION}" - - # Install designated SQLAlchemy version. - if [ ${SQLALCHEMY_VERSION} = "latest" ]; then - pip install "sqlalchemy" --upgrade - else - pip install "sqlalchemy==${SQLALCHEMY_VERSION}" - fi # Replace CrateDB version. if [ ${CRATEDB_VERSION} = "nightly" ]; then From 36a99cad03b5af536308c503d150fc216209ccae Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Thu, 22 Dec 2022 13:09:52 +0100 Subject: [PATCH 2/2] Tests: Fix `PGCompiler` SQL renderer anomaly on limit clauses 4532ef79 started using the `PGCompiler` for generating limit clauses, to be more compatible with CrateDB on edge cases. Apparently, the SQL renderer of `PGCompiler` was slightly changed with SQLAlchemy 1.4, which removes a redundant space character between table name and limit clause. This slipped through because CI did not test on SA13 appropriately at the time the change was added. --- src/crate/client/sqlalchemy/tests/compiler_test.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/crate/client/sqlalchemy/tests/compiler_test.py b/src/crate/client/sqlalchemy/tests/compiler_test.py index 62157d57..c49e14b3 100644 --- a/src/crate/client/sqlalchemy/tests/compiler_test.py +++ b/src/crate/client/sqlalchemy/tests/compiler_test.py @@ -26,6 +26,7 @@ import sqlalchemy as sa from sqlalchemy.sql import update, text +from crate.client.sqlalchemy.sa_version import SA_VERSION, SA_1_4 from crate.client.sqlalchemy.types import Craty @@ -77,7 +78,10 @@ def test_select_with_offset(self): self.metadata.bind = self.crate_engine selectable = self.mytable.select().offset(5) statement = str(selectable.compile()) - self.assertEqual(statement, "SELECT mytable.name, mytable.data \nFROM mytable\n LIMIT ALL OFFSET ?") + if SA_VERSION >= SA_1_4: + self.assertEqual(statement, "SELECT mytable.name, mytable.data \nFROM mytable\n LIMIT ALL OFFSET ?") + else: + self.assertEqual(statement, "SELECT mytable.name, mytable.data \nFROM mytable \n LIMIT ALL OFFSET ?") def test_select_with_limit(self): """