From 19484104556878caea49ce9c69168c16ff3a6c64 Mon Sep 17 00:00:00 2001 From: Ivo Dilov Date: Tue, 5 Nov 2024 11:33:15 +0200 Subject: [PATCH 1/4] Skip azure compat tests because of #1979 Also since 4.5.1 is now available we replace 4.5.0 with 4.5.1 which should re-enable the lmdb and mongo tests with their segfaults fixed. --- python/tests/compat/conftest.py | 6 ++++-- python/tests/compat/requirements-4.5.0.txt | 3 --- python/tests/compat/requirements-4.5.1.txt | 3 +++ 3 files changed, 7 insertions(+), 5 deletions(-) delete mode 100644 python/tests/compat/requirements-4.5.0.txt create mode 100644 python/tests/compat/requirements-4.5.1.txt diff --git a/python/tests/compat/conftest.py b/python/tests/compat/conftest.py index 90691479d1..4ae62c49ee 100644 --- a/python/tests/compat/conftest.py +++ b/python/tests/compat/conftest.py @@ -140,7 +140,7 @@ def assert_read(self, sym : str, df) -> None: scope="session", params=[ pytest.param("1.6.2", marks=VENV_COMPAT_TESTS_MARK), - pytest.param("4.5.0", marks=VENV_COMPAT_TESTS_MARK), + pytest.param("4.5.1", marks=VENV_COMPAT_TESTS_MARK), ] # TODO: Extend this list with other old versions ) def old_venv(request): @@ -176,10 +176,12 @@ def arctic_uri(request): @pytest.fixture() def old_venv_and_arctic_uri(old_venv, arctic_uri): - # TODO: Replace 4.5.0 with 4.5.1 when it is released to re-enable both mongo and lmdb. if Version(old_venv.version) <= Version("4.5.0") and arctic_uri.startswith("mongo"): pytest.skip("Mongo storage backend has a desctruction bug present until 4.5.0, which can cause flaky segfaults.") if Version(old_venv.version) <= Version("4.5.0") and arctic_uri.startswith("lmdb"): pytest.skip("LMDB storage backend has a desctruction bug present until 4.5.0, which can cause flaky segfaults.") + if arctic_uri.startswith("azure"): + # TODO: Once #1979 is understood and fixed reenable azure on versions which have the fix. + pytest.skip("Azure storage backend has probable a desctruction bug, which can cause flaky segfaults.") return old_venv, arctic_uri diff --git a/python/tests/compat/requirements-4.5.0.txt b/python/tests/compat/requirements-4.5.0.txt deleted file mode 100644 index 84e0dd5bea..0000000000 --- a/python/tests/compat/requirements-4.5.0.txt +++ /dev/null @@ -1,3 +0,0 @@ -arcticdb==4.5.0 -numpy<2 -pyarrow \ No newline at end of file diff --git a/python/tests/compat/requirements-4.5.1.txt b/python/tests/compat/requirements-4.5.1.txt new file mode 100644 index 0000000000..e10c212307 --- /dev/null +++ b/python/tests/compat/requirements-4.5.1.txt @@ -0,0 +1,3 @@ +arcticdb==4.5.1 +numpy<2 +pyarrow \ No newline at end of file From 2c996a879e999aa7ca6be4d0cf26af64afed079d Mon Sep 17 00:00:00 2001 From: Ivo Dilov Date: Tue, 5 Nov 2024 11:39:04 +0200 Subject: [PATCH 2/4] Use `shell=True` for linux compat tests to fix feedstock runs The issue with conda feedstock runners is they use some variable for abspath, which doesn't get correctly expanded by `os.path.expandvar`. This requires using `shell=True` for linux as well. This was tested to properly fix compat tests in #1931 combined with [feedstock pr](https://github.com/conda-forge/arcticdb-feedstock/pull/322). However it still doesn't work for some of the macos builds which are not skipped correctly. Still this is a good change for local runs as well and we'll fix the macos issue in a separate run. --- python/tests/compat/conftest.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/python/tests/compat/conftest.py b/python/tests/compat/conftest.py index 4ae62c49ee..948430dc21 100644 --- a/python/tests/compat/conftest.py +++ b/python/tests/compat/conftest.py @@ -23,8 +23,15 @@ def is_running_on_windows(): def run_shell_command(command : List[Union[str, os.PathLike]], cwd : Optional[os.PathLike] = None) -> subprocess.CompletedProcess: logger.info(f"Executing command: {command}") - # shell=True is required for running the correct python executable on Windows - result = subprocess.run(command, cwd=cwd, capture_output=True, shell=is_running_on_windows()) + result = None + if is_running_on_windows(): + # shell=True is required for running the correct python executable on Windows + result = subprocess.run(command, cwd=cwd, capture_output=True, shell=True) + else: + # On linux we need shell=True for conda feedstock runners (because otherwise they fail to expand path variables) + # But to correctly work with shell=True we need a single command string. + command_string = ' '.join(command) + result = subprocess.run(command_string, cwd=cwd, capture_output=True, shell=True, stdin=subprocess.DEVNULL) if result.returncode != 0: logger.warning(f"Command failed, stdout: {str(result.stdout)}, stderr: {str(result.stderr)}") return result @@ -146,9 +153,8 @@ def assert_read(self, sym : str, df) -> None: def old_venv(request): version = request.param path = os.path.join("venvs", version) - # The requirements_file needs to be relative to the [path] we use for the venv. - # Absolute paths break some Azure CI runners on conda forge - requirements_file = os.path.join("..", "..", "tests", "compat", f"requirements-{version}.txt") + compat_dir = os.path.dirname(os.path.abspath(__file__)) + requirements_file = os.path.join(compat_dir, f"requirements-{version}.txt") with Venv(path, requirements_file, version) as old_venv: yield old_venv From c8c1f7dbfa09818b63bfd9cd8e8367dbc5504284 Mon Sep 17 00:00:00 2001 From: Ivo Dilov Date: Tue, 5 Nov 2024 14:11:12 +0200 Subject: [PATCH 3/4] Skip mongo tests entirely The destruction bug on mongo I thought I fixed in #1862 can still be seen. We skip the mongo test for now. --- python/tests/compat/conftest.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/tests/compat/conftest.py b/python/tests/compat/conftest.py index 948430dc21..d9a3bb0ce8 100644 --- a/python/tests/compat/conftest.py +++ b/python/tests/compat/conftest.py @@ -182,8 +182,9 @@ def arctic_uri(request): @pytest.fixture() def old_venv_and_arctic_uri(old_venv, arctic_uri): - if Version(old_venv.version) <= Version("4.5.0") and arctic_uri.startswith("mongo"): - pytest.skip("Mongo storage backend has a desctruction bug present until 4.5.0, which can cause flaky segfaults.") + if arctic_uri.startswith("mongo"): + # TODO: We tought the bug was fixed in 4.5.1 but it's still present. + pytest.skip("Mongo storage backend has a desctruction bug, which can cause flaky segfaults.") if Version(old_venv.version) <= Version("4.5.0") and arctic_uri.startswith("lmdb"): pytest.skip("LMDB storage backend has a desctruction bug present until 4.5.0, which can cause flaky segfaults.") if arctic_uri.startswith("azure"): From 7941c47ea14ab602e7365100b5d6ce6feeaf0dd5 Mon Sep 17 00:00:00 2001 From: Ivo Dilov Date: Tue, 5 Nov 2024 17:06:49 +0200 Subject: [PATCH 4/4] Re comments on #1979 we skip the lmdb compat tests as well After we fix the segfaults we can re-enable --- python/tests/compat/conftest.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/python/tests/compat/conftest.py b/python/tests/compat/conftest.py index d9a3bb0ce8..202b5f8117 100644 --- a/python/tests/compat/conftest.py +++ b/python/tests/compat/conftest.py @@ -182,13 +182,12 @@ def arctic_uri(request): @pytest.fixture() def old_venv_and_arctic_uri(old_venv, arctic_uri): + # TODO: Once #1979 is understood and fixed reenable mongo, lmdb and azure for versions which have the fix. if arctic_uri.startswith("mongo"): - # TODO: We tought the bug was fixed in 4.5.1 but it's still present. - pytest.skip("Mongo storage backend has a desctruction bug, which can cause flaky segfaults.") - if Version(old_venv.version) <= Version("4.5.0") and arctic_uri.startswith("lmdb"): - pytest.skip("LMDB storage backend has a desctruction bug present until 4.5.0, which can cause flaky segfaults.") + pytest.skip("Mongo storage backend has a probable desctruction bug, which can cause flaky segfaults.") + if arctic_uri.startswith("lmdb"): + pytest.skip("LMDB storage backend has a probable desctruction bug, which can cause flaky segfaults.") if arctic_uri.startswith("azure"): - # TODO: Once #1979 is understood and fixed reenable azure on versions which have the fix. pytest.skip("Azure storage backend has probable a desctruction bug, which can cause flaky segfaults.") return old_venv, arctic_uri