From: Daniele Varrazzo Date: Mon, 15 Aug 2022 11:52:04 +0000 (+0200) Subject: fix: consider pipeline mode not supported with libpq 14.5 X-Git-Tag: 3.1~20 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=923bb834ce7601f2743d10bff7e007698429933a;p=thirdparty%2Fpsycopg.git fix: consider pipeline mode not supported with libpq 14.5 Improve diagnostics about the reason why the pipeline mode is not supported. Improve not supported tests. --- diff --git a/psycopg/psycopg/_pipeline.py b/psycopg/psycopg/_pipeline.py index ac7d25efe..6b941bf28 100644 --- a/psycopg/psycopg/_pipeline.py +++ b/psycopg/psycopg/_pipeline.py @@ -56,18 +56,44 @@ class BasePipeline: def status(self) -> pq.PipelineStatus: return pq.PipelineStatus(self.pgconn.pipeline_status) - @staticmethod - def is_supported() -> bool: + @classmethod + def is_supported(cls) -> bool: """Return `!True` if the psycopg libpq wrapper supports pipeline mode.""" if BasePipeline._is_supported is None: - # Support only depends on the libpq functions available in the pq - # wrapper, not on the database version. - pq_version = pq.__build_version__ or pq.version() - # Pipeline support broken in libpq 14.5 (#350) - BasePipeline._is_supported = pq_version >= 140000 and pq_version != 140005 + BasePipeline._is_supported = not cls._not_supported_reason() return BasePipeline._is_supported + @classmethod + def _not_supported_reason(cls) -> str: + """Return the reason why the pipeline mode is not supported. + + Return an empty string if pipeline mode is supported. + """ + # Support only depends on the libpq functions available in the pq + # wrapper, not on the database version. + if pq.version() < 140000: + return ( + f"libpq too old {pq.version()};" + " v14 or greater required for pipeline mode" + ) + + if pq.__build_version__ < 140000: + return ( + f"libpq too old: module built for {pq.__build_version__};" + " v14 or greater required for pipeline mode" + ) + + # Bug #350 + if pq.version() == 140005: + return f"pipeline mode broken in libpq version {pq.version()}" + + return "" + def _enter_gen(self) -> PQGen[None]: + if not self.is_supported(): + raise e.NotSupportedError( + f"pipeline mode not supported: {self._not_supported_reason()}" + ) if self.level == 0: self.pgconn.enter_pipeline_mode() elif self.command_queue: diff --git a/tests/conftest.py b/tests/conftest.py index 9d691b12d..598c36aa4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -26,7 +26,6 @@ def pytest_configure(config): "timing: the test is timing based and can fail on cheese hardware", "dns: the test requires dnspython to run", "postgis: the test requires the PostGIS extension to run", - "pipeline: the test runs with connection in pipeline mode", ] for marker in markers: diff --git a/tests/fix_db.py b/tests/fix_db.py index ded17f710..11b6c2520 100644 --- a/tests/fix_db.py +++ b/tests/fix_db.py @@ -8,7 +8,7 @@ import psycopg from psycopg import pq from psycopg import sql -from .utils import check_libpq_version, check_postgres_version +from .utils import check_postgres_version # Set by warm_up_database() the first time the dsn fixture is used pg_version: int @@ -57,13 +57,21 @@ def pytest_collection_modifyitems(items): break +def pytest_runtest_setup(item): + for m in item.iter_markers(name="pipeline"): + if not psycopg.Pipeline.is_supported(): + pytest.skip(psycopg.Pipeline._not_supported_reason()) + + def pytest_configure(config): # register pg marker - config.addinivalue_line( - "markers", + markers = [ "pg(version_expr): run the test only with matching server version" " (e.g. '>= 10', '< 9.6')", - ) + "pipeline: the test runs with connection in pipeline mode", + ] + for marker in markers: + config.addinivalue_line("markers", marker) @pytest.fixture(scope="session") @@ -160,9 +168,8 @@ def conn(conn_cls, dsn, request, tracefile): @pytest.fixture(params=[True, False], ids=["pipeline=on", "pipeline=off"]) def pipeline(request, conn): if request.param: - msg = check_libpq_version(pq.version(), ">= 14") - if msg: - pytest.skip(msg) + if not psycopg.Pipeline.is_supported(): + pytest.skip(psycopg.Pipeline._not_supported_reason()) with conn.pipeline() as p: yield p return @@ -184,9 +191,8 @@ async def aconn(dsn, aconn_cls, request, tracefile): @pytest.fixture(params=[True, False], ids=["pipeline=on", "pipeline=off"]) async def apipeline(request, aconn): if request.param: - msg = check_libpq_version(pq.version(), ">= 14") - if msg: - pytest.skip(msg) + if not psycopg.Pipeline.is_supported(): + pytest.skip(psycopg.Pipeline._not_supported_reason()) async with aconn.pipeline() as p: yield p return diff --git a/tests/pq/test_pq.py b/tests/pq/test_pq.py index 8cceace56..a37fff333 100644 --- a/tests/pq/test_pq.py +++ b/tests/pq/test_pq.py @@ -32,10 +32,41 @@ def test_want_import_version(): assert not check_libpq_version(got, want) -def test_pipeline_supported(): - # Note: This test is here because pipeline tests are skipped on libpq < 14 - if pq.__impl__ == "python": - assert psycopg.Pipeline.is_supported() == (pq.version() >= 140000) - else: - assert pq.__build_version__ is not None - assert psycopg.Pipeline.is_supported() == (pq.__build_version__ >= 140000) +# Note: These tests are here because test_pipeline.py tests are all skipped +# when pipeline mode is not supported. + + +@pytest.mark.libpq(">= 14") +@pytest.mark.libpq("!= 14.5") +def test_pipeline_supported(conn): + assert psycopg.Pipeline.is_supported() + assert psycopg.AsyncPipeline.is_supported() + + with conn.pipeline(): + pass + + +@pytest.mark.libpq("< 14") +def test_pipeline_not_supported(conn): + assert not psycopg.Pipeline.is_supported() + assert not psycopg.AsyncPipeline.is_supported() + + with pytest.raises(psycopg.NotSupportedError) as exc: + with conn.pipeline(): + pass + + assert "too old" in str(exc.value) + + +@pytest.mark.libpq("14.5") +def test_pipeline_not_supported_14_5(conn): + # Affected by #350 + # NOTE: we might support it in binary using a patched libpq version. + assert not psycopg.Pipeline.is_supported() + assert not psycopg.AsyncPipeline.is_supported() + + with pytest.raises(psycopg.NotSupportedError) as exc: + with conn.pipeline(): + pass + + assert "broken" in str(exc.value) diff --git a/tests/test_client_cursor.py b/tests/test_client_cursor.py index fd733984b..42242f655 100644 --- a/tests/test_client_cursor.py +++ b/tests/test_client_cursor.py @@ -818,7 +818,6 @@ def test_mogrify_badenc(conn, encoding): conn.cursor().mogrify("select %(s)s", {"s": "\u20ac"}) -@pytest.mark.libpq(">= 14") @pytest.mark.pipeline def test_message_0x33(conn): # https://github.com/psycopg/psycopg/issues/314 diff --git a/tests/test_client_cursor_async.py b/tests/test_client_cursor_async.py index adcea69c6..35156b64d 100644 --- a/tests/test_client_cursor_async.py +++ b/tests/test_client_cursor_async.py @@ -690,7 +690,6 @@ async def test_mogrify_badenc(aconn, encoding): aconn.cursor().mogrify("select %(s)s", {"s": "\u20ac"}) -@pytest.mark.libpq(">= 14") @pytest.mark.pipeline async def test_message_0x33(aconn): # https://github.com/psycopg/psycopg/issues/314 diff --git a/tests/test_conninfo.py b/tests/test_conninfo.py index 60983e31b..296ec0604 100644 --- a/tests/test_conninfo.py +++ b/tests/test_conninfo.py @@ -177,7 +177,7 @@ class TestConnectionInfo: conn.close() assert conn.info.transaction_status.name == "UNKNOWN" - @pytest.mark.libpq(">= 14") + @pytest.mark.pipeline def test_pipeline_status(self, conn): assert not conn.info.pipeline_status assert conn.info.pipeline_status.name == "OFF" diff --git a/tests/test_generators.py b/tests/test_generators.py index b47c620a1..c396d0425 100644 --- a/tests/test_generators.py +++ b/tests/test_generators.py @@ -4,16 +4,13 @@ from typing import List import pytest +import psycopg from psycopg import waiting from psycopg import pq -from .utils import check_libpq_version - @pytest.fixture def pipeline(pgconn): - if check_libpq_version(pq.version(), ">= 14"): - pytest.skip("require libpq >= 14") nb, pgconn.nonblocking = pgconn.nonblocking, True assert pgconn.nonblocking pgconn.enter_pipeline_mode() @@ -40,6 +37,7 @@ def _run_pipeline_communicate(pgconn, generators, commands, expected_statuses): assert actual_statuses == expected_statuses +@pytest.mark.pipeline def test_pipeline_communicate_multi_pipeline(pgconn, pipeline, generators): commands = deque( [ @@ -58,6 +56,7 @@ def test_pipeline_communicate_multi_pipeline(pgconn, pipeline, generators): _run_pipeline_communicate(pgconn, generators, commands, expected_statuses) +@pytest.mark.pipeline def test_pipeline_communicate_no_sync(pgconn, pipeline, generators): numqueries = 10 commands = deque( @@ -70,8 +69,6 @@ def test_pipeline_communicate_no_sync(pgconn, pipeline, generators): @pytest.fixture def pipeline_demo(pgconn): - if check_libpq_version(pq.version(), ">= 14"): - pytest.skip("require libpq >= 14") assert pgconn.pipeline_status == 0 res = pgconn.exec_(b"DROP TABLE IF EXISTS pg_pipeline") assert res.status == pq.ExecStatus.COMMAND_OK, res.error_message @@ -85,6 +82,7 @@ def pipeline_demo(pgconn): # TODOCRDB: 1 doesn't get rolled back. Open a ticket? +@pytest.mark.pipeline @pytest.mark.crdb("skip", reason="pipeline aborted") def test_pipeline_communicate_abort(pgconn, pipeline_demo, pipeline, generators): insert_sql = b"insert into pg_pipeline(itemno) values ($1)" @@ -115,8 +113,8 @@ def test_pipeline_communicate_abort(pgconn, pipeline_demo, pipeline, generators) @pytest.fixture def pipeline_uniqviol(pgconn): - if check_libpq_version(pq.version(), ">= 14"): - pytest.skip("require libpq >= 14") + if not psycopg.Pipeline.is_supported(): + pytest.skip(psycopg.Pipeline._not_supported_reason()) assert pgconn.pipeline_status == 0 res = pgconn.exec_(b"DROP TABLE IF EXISTS pg_pipeline_uniqviol") assert res.status == pq.ExecStatus.COMMAND_OK, res.error_message diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index c508bc64c..3a239e55b 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -11,8 +11,8 @@ from psycopg import pq from psycopg import errors as e pytestmark = [ - pytest.mark.libpq(">= 14"), pytest.mark.pipeline, + pytest.mark.skipif("not psycopg.Pipeline.is_supported()"), ] pipeline_aborted = pytest.mark.flakey("the server might get in pipeline aborted") diff --git a/tests/test_pipeline_async.py b/tests/test_pipeline_async.py index 57343b1a5..0119aae94 100644 --- a/tests/test_pipeline_async.py +++ b/tests/test_pipeline_async.py @@ -14,8 +14,8 @@ from .test_pipeline import pipeline_aborted pytestmark = [ pytest.mark.asyncio, - pytest.mark.libpq(">= 14"), pytest.mark.pipeline, + pytest.mark.skipif("not psycopg.AsyncPipeline.is_supported()"), ]