From 4db17e4cdbdec3e0460920d2efe4696a7c6d5efd Mon Sep 17 00:00:00 2001 From: Erin Drummond Date: Thu, 28 Aug 2025 23:12:17 +0000 Subject: [PATCH 1/3] Chore: Address more integration test flakiness --- .circleci/manage-test-db.sh | 6 +++- sqlmesh/core/engine_adapter/fabric.py | 6 ++++ tests/conftest.py | 45 +++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/.circleci/manage-test-db.sh b/.circleci/manage-test-db.sh index ba1d1070fb..f79072f335 100755 --- a/.circleci/manage-test-db.sh +++ b/.circleci/manage-test-db.sh @@ -80,7 +80,11 @@ redshift_down() { EXIT_CODE=1 ATTEMPTS=0 while [ $EXIT_CODE -ne 0 ] && [ $ATTEMPTS -lt 5 ]; do - redshift_exec "select pg_terminate_backend(procpid) from pg_stat_activity where datname = '$1'" + # note: sometimes this pg_terminate_backend() call can randomly fail with: ERROR: Insufficient privileges + # if it does, let's proceed with the drop anyway rather than aborting and never attempting the drop + redshift_exec "select pg_terminate_backend(procpid) from pg_stat_activity where datname = '$1'" || true + + # perform drop redshift_exec "drop database $1;" && EXIT_CODE=$? || EXIT_CODE=$? if [ $EXIT_CODE -ne 0 ]; then echo "Unable to drop database; retrying..." diff --git a/sqlmesh/core/engine_adapter/fabric.py b/sqlmesh/core/engine_adapter/fabric.py index 6d7d40c3bb..585622e866 100644 --- a/sqlmesh/core/engine_adapter/fabric.py +++ b/sqlmesh/core/engine_adapter/fabric.py @@ -153,6 +153,12 @@ def set_current_catalog(self, catalog_name: str) -> None: logger.info(f"Switching from catalog '{current_catalog}' to '{catalog_name}'") + # commit the transaction before closing the connection to help prevent errors like: + # > Snapshot isolation transaction failed in database because the object accessed by the statement has been modified by a + # > DDL statement in another concurrent transaction since the start of this transaction + # on subsequent queries in the new connection + self._connection_pool.commit() + # note: we call close() on the connection pool instead of self.close() because self.close() calls close_all() # on the connection pool but we just want to close the connection for this thread self._connection_pool.close() diff --git a/tests/conftest.py b/tests/conftest.py index e4911de80c..b6523f72a4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -245,6 +245,51 @@ def pytest_runtest_makereport(item: pytest.Item, call: pytest.CallInfo): ) +def pytest_configure(config: pytest.Config): + # we need to adjust the hook order if pytest-retry is present because it: + # - also declares a `pytest_runtest_makereport` with `hookwrapper=True, tryfirst=True` + # - this supersedes our one because pytest always loads plugins first and they take precedence over user code + # + # but, we need our one to run first because it's capturing and ignoring certain errors that cause pytest-retry to fail + # and not retry. so we need to adjust the order the hooks are called which pytest does NOT make easy. + # + # we can't just unload the pytest-retry plugin, load our hook and reload the pytest-retry plugin either. + # this causes an error: + # > Hook 'pytest_set_excluded_exceptions' is already registered within namespace + # because unregister() apparently doesnt unregister plugins cleanly in such a way they can be re-registered + # + # so what we end up doing below is a small monkey-patch to adjust the call order of the hooks + pm = config.pluginmanager + + from pluggy._hooks import HookCaller + + hook_caller: HookCaller = pm.hook.pytest_runtest_makereport + hook_impls = hook_caller.get_hookimpls() + + # find the index of our one + our_makereport_idx = next( + (i for i, v in enumerate(hook_impls) if v.plugin_name.endswith("tests/conftest.py")), None + ) + + # find the index of the pytest-retry one + pytest_retry_makereport_idx = next( + (i for i, v in enumerate(hook_impls) if v.plugin_name == "pytest-retry"), None + ) + + if ( + pytest_retry_makereport_idx is not None + and our_makereport_idx is not None + and our_makereport_idx > pytest_retry_makereport_idx + ): + our_makereport_hook = hook_impls.pop(our_makereport_idx) + + # inject our one to run before the pytest-retry one + hook_impls.insert(pytest_retry_makereport_idx, our_makereport_hook) + + # HookCaller doesnt have a setter method for this. + hook_caller._hookimpls = hook_impls # type: ignore + + # Ignore all local config files @pytest.fixture(scope="session", autouse=True) def ignore_local_config_files(): From f455332b5cd542d566d9bb7a64c9fc494f4379f1 Mon Sep 17 00:00:00 2001 From: Erin Drummond Date: Thu, 28 Aug 2025 23:12:46 +0000 Subject: [PATCH 2/3] Enable cloud tests --- .circleci/continue_config.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.circleci/continue_config.yml b/.circleci/continue_config.yml index e21f3d869b..40d7c92974 100644 --- a/.circleci/continue_config.yml +++ b/.circleci/continue_config.yml @@ -310,10 +310,10 @@ workflows: - athena - fabric - gcp-postgres - filters: - branches: - only: - - main + #filters: + # branches: + # only: + # - main - ui_style - ui_test - vscode_test From abd2da6573cfd8d1f7609d8821b1f3d8b7384c59 Mon Sep 17 00:00:00 2001 From: Erin Drummond Date: Thu, 28 Aug 2025 23:48:23 +0000 Subject: [PATCH 3/3] Revert "Enable cloud tests" This reverts commit f455332b5cd542d566d9bb7a64c9fc494f4379f1. --- .circleci/continue_config.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.circleci/continue_config.yml b/.circleci/continue_config.yml index 40d7c92974..e21f3d869b 100644 --- a/.circleci/continue_config.yml +++ b/.circleci/continue_config.yml @@ -310,10 +310,10 @@ workflows: - athena - fabric - gcp-postgres - #filters: - # branches: - # only: - # - main + filters: + branches: + only: + - main - ui_style - ui_test - vscode_test