From b9444054260eb10c7d473dc55cb7cd7e8a0e84b6 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 19 Jun 2023 05:02:20 +0000 Subject: [PATCH 01/11] fix: motherduck_token --- duckdb_engine/config.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/duckdb_engine/config.py b/duckdb_engine/config.py index a32f72ef..e458565c 100644 --- a/duckdb_engine/config.py +++ b/duckdb_engine/config.py @@ -16,7 +16,8 @@ def get_core_config() -> Set[str]: .execute("SELECT name FROM duckdb_settings()") .fetchall() ) - return {name for name, in rows} + # special case for motherduck here - they accept this config at extension load time + return {name for name, in rows} | {'motherduck_token'} def apply_config( From 8eea0dd72bc295258dfc915a141c247c26f0b3eb Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 19 Jun 2023 05:26:11 +0000 Subject: [PATCH 02/11] test: add very basic motherduck test --- duckdb_engine/config.py | 2 +- duckdb_engine/tests/test_integration.py | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/duckdb_engine/config.py b/duckdb_engine/config.py index e458565c..41b0c650 100644 --- a/duckdb_engine/config.py +++ b/duckdb_engine/config.py @@ -17,7 +17,7 @@ def get_core_config() -> Set[str]: .fetchall() ) # special case for motherduck here - they accept this config at extension load time - return {name for name, in rows} | {'motherduck_token'} + return {name for name, in rows} | {"motherduck_token"} def apply_config( diff --git a/duckdb_engine/tests/test_integration.py b/duckdb_engine/tests/test_integration.py index 2c14e4a8..52cc5f96 100644 --- a/duckdb_engine/tests/test_integration.py +++ b/duckdb_engine/tests/test_integration.py @@ -1,6 +1,8 @@ import pandas as pd +from pytest import mark, raises from sqlalchemy import text -from sqlalchemy.engine import Engine +from sqlalchemy.engine import Engine, create_engine +from sqlalchemy.exc import ProgrammingError def test_integration(engine: Engine) -> None: @@ -12,3 +14,15 @@ def test_integration(engine: Engine) -> None: execute("register", params) # type: ignore[operator] conn.execute(text("select * from test_df")) + + +@mark.remote_data +def test_motherduck(): + engine = create_engine("duckdb:///md:motherdb?motherduck_token=motherduckdb_token") + + with raises( + ProgrammingError, + match="Jwt is not in the form of Header.Payload.Signature with two dots and 3 sections", + ): + with engine.connect() as conn: + pass From fa013f826a02a4b934e77788557043525568eba8 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 19 Jun 2023 14:40:24 +0800 Subject: [PATCH 03/11] chore: missing type annotation --- duckdb_engine/tests/test_integration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/duckdb_engine/tests/test_integration.py b/duckdb_engine/tests/test_integration.py index 52cc5f96..f60007e4 100644 --- a/duckdb_engine/tests/test_integration.py +++ b/duckdb_engine/tests/test_integration.py @@ -17,7 +17,7 @@ def test_integration(engine: Engine) -> None: @mark.remote_data -def test_motherduck(): +def test_motherduck() -> None: engine = create_engine("duckdb:///md:motherdb?motherduck_token=motherduckdb_token") with raises( From 6173fbf9b26efeb54f7853fbe07ddbae937e84ae Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 19 Jun 2023 14:58:44 +0800 Subject: [PATCH 04/11] test: skip md test on dev builds --- duckdb_engine/tests/test_integration.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/duckdb_engine/tests/test_integration.py b/duckdb_engine/tests/test_integration.py index f60007e4..3e18f85d 100644 --- a/duckdb_engine/tests/test_integration.py +++ b/duckdb_engine/tests/test_integration.py @@ -1,3 +1,4 @@ +import duckdb import pandas as pd from pytest import mark, raises from sqlalchemy import text @@ -17,6 +18,9 @@ def test_integration(engine: Engine) -> None: @mark.remote_data +@mark.skipif( + "dev" in duckdb.__version__, reason="md extension not available for dev builds" +) def test_motherduck() -> None: engine = create_engine("duckdb:///md:motherdb?motherduck_token=motherduckdb_token") From 8c0c780a604b6775e10a375fbea56630750eca78 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 19 Jun 2023 15:07:00 +0800 Subject: [PATCH 05/11] chore: ignore type error --- duckdb_engine/tests/test_integration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/duckdb_engine/tests/test_integration.py b/duckdb_engine/tests/test_integration.py index 3e18f85d..33fcc5e8 100644 --- a/duckdb_engine/tests/test_integration.py +++ b/duckdb_engine/tests/test_integration.py @@ -19,7 +19,7 @@ def test_integration(engine: Engine) -> None: @mark.remote_data @mark.skipif( - "dev" in duckdb.__version__, reason="md extension not available for dev builds" + "dev" in duckdb.__version__, reason="md extension not available for dev builds" # type: ignore[attr-defined] ) def test_motherduck() -> None: engine = create_engine("duckdb:///md:motherdb?motherduck_token=motherduckdb_token") From 31601c468633cac9653ddda7a10a734e43e3c403 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 19 Jun 2023 07:22:36 +0000 Subject: [PATCH 06/11] chore: remove with statement --- duckdb_engine/tests/test_integration.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/duckdb_engine/tests/test_integration.py b/duckdb_engine/tests/test_integration.py index 33fcc5e8..9ec11862 100644 --- a/duckdb_engine/tests/test_integration.py +++ b/duckdb_engine/tests/test_integration.py @@ -28,5 +28,4 @@ def test_motherduck() -> None: ProgrammingError, match="Jwt is not in the form of Header.Payload.Signature with two dots and 3 sections", ): - with engine.connect() as conn: - pass + engine.connect() From 9dd218b9b003bb1ac81604c86f3f63f925da770b Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 19 Jun 2023 22:45:49 +0800 Subject: [PATCH 07/11] test: connect_args should be used here until other pr is merged --- duckdb_engine/tests/test_integration.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/duckdb_engine/tests/test_integration.py b/duckdb_engine/tests/test_integration.py index 9ec11862..2a38fde0 100644 --- a/duckdb_engine/tests/test_integration.py +++ b/duckdb_engine/tests/test_integration.py @@ -22,7 +22,10 @@ def test_integration(engine: Engine) -> None: "dev" in duckdb.__version__, reason="md extension not available for dev builds" # type: ignore[attr-defined] ) def test_motherduck() -> None: - engine = create_engine("duckdb:///md:motherdb?motherduck_token=motherduckdb_token") + engine = create_engine( + "duckdb:///md:motherdb", + connect_args={"config": {"motherduck_token": "motherduckdb_token"}}, + ) with raises( ProgrammingError, From 27d6a13556407654873b3dda93f4936272f92ae0 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Mon, 19 Jun 2023 23:09:24 +0800 Subject: [PATCH 08/11] test: md extension is only available from duckdb 0.7.1 and up --- duckdb_engine/tests/test_integration.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/duckdb_engine/tests/test_integration.py b/duckdb_engine/tests/test_integration.py index 2a38fde0..5d44e206 100644 --- a/duckdb_engine/tests/test_integration.py +++ b/duckdb_engine/tests/test_integration.py @@ -1,6 +1,6 @@ import duckdb import pandas as pd -from pytest import mark, raises +from pytest import importorskip, mark, raises from sqlalchemy import text from sqlalchemy.engine import Engine, create_engine from sqlalchemy.exc import ProgrammingError @@ -22,6 +22,8 @@ def test_integration(engine: Engine) -> None: "dev" in duckdb.__version__, reason="md extension not available for dev builds" # type: ignore[attr-defined] ) def test_motherduck() -> None: + importorskip("duckdb", "0.7.1") + engine = create_engine( "duckdb:///md:motherdb", connect_args={"config": {"motherduck_token": "motherduckdb_token"}}, From 05fb12090e321949f4839c19f17bf96eb8426194 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Tue, 20 Jun 2023 09:58:22 +0000 Subject: [PATCH 09/11] test: run motherduck test in subprocess to work around segfaults --- duckdb_engine/tests/test_integration.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/duckdb_engine/tests/test_integration.py b/duckdb_engine/tests/test_integration.py index 5d44e206..276b81ea 100644 --- a/duckdb_engine/tests/test_integration.py +++ b/duckdb_engine/tests/test_integration.py @@ -1,3 +1,7 @@ +import sys +from multiprocessing import Process +from traceback import print_exc + import duckdb import pandas as pd from pytest import importorskip, mark, raises @@ -24,6 +28,23 @@ def test_integration(engine: Engine) -> None: def test_motherduck() -> None: importorskip("duckdb", "0.7.1") + proc = Process(target=hook) + proc.start() + proc.join() + assert proc.exitcode == 0 + + +def hook() -> None: + try: + _test_motherduck() + except Exception: + print_exc() + sys.exit(-1) + else: + sys.exit(0) + + +def _test_motherduck() -> None: engine = create_engine( "duckdb:///md:motherdb", connect_args={"config": {"motherduck_token": "motherduckdb_token"}}, From 7c018ce86564efe37fe007ea50a4479581b2af1c Mon Sep 17 00:00:00 2001 From: Elliana May Date: Tue, 20 Jun 2023 21:23:55 +0800 Subject: [PATCH 10/11] test: use new process instead of fork --- duckdb_engine/tests/test_integration.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/duckdb_engine/tests/test_integration.py b/duckdb_engine/tests/test_integration.py index 276b81ea..d63800a4 100644 --- a/duckdb_engine/tests/test_integration.py +++ b/duckdb_engine/tests/test_integration.py @@ -1,5 +1,5 @@ import sys -from multiprocessing import Process +from subprocess import Popen from traceback import print_exc import duckdb @@ -28,10 +28,8 @@ def test_integration(engine: Engine) -> None: def test_motherduck() -> None: importorskip("duckdb", "0.7.1") - proc = Process(target=hook) - proc.start() - proc.join() - assert proc.exitcode == 0 + proc = Popen([sys.executable, __file__]) + assert proc.wait(5000) == 0 def hook() -> None: @@ -55,3 +53,7 @@ def _test_motherduck() -> None: match="Jwt is not in the form of Header.Payload.Signature with two dots and 3 sections", ): engine.connect() + + +if __name__ == "__main__": + _test_motherduck() From d4a56fcec608f405478ae149d378a99c693a2df7 Mon Sep 17 00:00:00 2001 From: Elliana May Date: Tue, 20 Jun 2023 21:52:42 +0800 Subject: [PATCH 11/11] fix: allow for segfaults too --- duckdb_engine/tests/test_integration.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/duckdb_engine/tests/test_integration.py b/duckdb_engine/tests/test_integration.py index d63800a4..349c976c 100644 --- a/duckdb_engine/tests/test_integration.py +++ b/duckdb_engine/tests/test_integration.py @@ -1,5 +1,5 @@ import sys -from subprocess import Popen +from subprocess import PIPE, Popen from traceback import print_exc import duckdb @@ -9,6 +9,9 @@ from sqlalchemy.engine import Engine, create_engine from sqlalchemy.exc import ProgrammingError +SEGFAULT = -6 +SUCCESS = 0 + def test_integration(engine: Engine) -> None: with engine.connect() as conn: @@ -28,8 +31,11 @@ def test_integration(engine: Engine) -> None: def test_motherduck() -> None: importorskip("duckdb", "0.7.1") - proc = Popen([sys.executable, __file__]) - assert proc.wait(5000) == 0 + # we're running this test in a sub process due to occasional segfaults in the motherduck extension + + proc = Popen([sys.executable, __file__], stderr=PIPE, stdout=PIPE, text=True) + wait = proc.wait(5000) + assert wait == SUCCESS or wait == SEGFAULT, (proc.stdout, proc.stderr) def hook() -> None: