From b023103d12285e1178b8a96a4ef472811ce7af30 Mon Sep 17 00:00:00 2001 From: w3lld1 <42353747+w3lld1@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:31:12 +0200 Subject: [PATCH] Deprecate DatabaseJanitor version argument - refs #1393 --- README.rst | 2 - newsfragments/1393.depr.rst | 1 + pytest_postgresql/factories/client.py | 2 - pytest_postgresql/factories/noprocess.py | 1 - pytest_postgresql/factories/process.py | 1 - pytest_postgresql/janitor.py | 24 ++++++---- tests/test_janitor.py | 60 +++++++++++------------- tests/test_noopexecutor.py | 1 - tests/test_postgres_options_plugin.py | 3 -- 9 files changed, 43 insertions(+), 52 deletions(-) create mode 100644 newsfragments/1393.depr.rst diff --git a/README.rst b/README.rst index 4ab3656f..fcfc4598 100644 --- a/README.rst +++ b/README.rst @@ -496,7 +496,6 @@ Advanced Usage: DatabaseJanitor host=postgresql_proc.host, port=postgresql_proc.port, dbname="my_custom_db", - version=postgresql_proc.version, password="secret_password", ): with psycopg.connect( @@ -531,7 +530,6 @@ fixtures. It requires ``psycopg`` (a core dependency). Install host=postgresql_proc.host, port=postgresql_proc.port, dbname="my_custom_db", - version=postgresql_proc.version, password="secret_password", ): async with await psycopg.AsyncConnection.connect( diff --git a/newsfragments/1393.depr.rst b/newsfragments/1393.depr.rst new file mode 100644 index 00000000..299425a9 --- /dev/null +++ b/newsfragments/1393.depr.rst @@ -0,0 +1 @@ +Deprecated the unused ``version`` argument of ``DatabaseJanitor`` and ``AsyncDatabaseJanitor``. diff --git a/pytest_postgresql/factories/client.py b/pytest_postgresql/factories/client.py index 6a6572f4..2cfc0bc4 100644 --- a/pytest_postgresql/factories/client.py +++ b/pytest_postgresql/factories/client.py @@ -93,7 +93,6 @@ def postgresql_factory(request: FixtureRequest) -> Iterator[Connection]: dbname=pg_db, template_dbname=proc_fixture.template_dbname, maintenance_dbname=proc_fixture.maintenance_dbname, - version=proc_fixture.version, password=pg_password, isolation_level=isolation_level, ) @@ -159,7 +158,6 @@ async def postgresql_async_factory(request: FixtureRequest) -> AsyncIterator[Asy dbname=pg_db, template_dbname=proc_fixture.template_dbname, maintenance_dbname=proc_fixture.maintenance_dbname, - version=proc_fixture.version, password=pg_password, isolation_level=isolation_level, ) diff --git a/pytest_postgresql/factories/noprocess.py b/pytest_postgresql/factories/noprocess.py index 5432ab48..66de9c8a 100644 --- a/pytest_postgresql/factories/noprocess.py +++ b/pytest_postgresql/factories/noprocess.py @@ -130,7 +130,6 @@ def postgresql_noproc_fixture(request: FixtureRequest) -> Iterator[NoopExecutor] template_dbname=base_template_dbname, maintenance_dbname=noop_exec.maintenance_dbname, as_template=True, - version=noop_exec.version, password=noop_exec.password, autocommit=janitor_load_autocommit, ) diff --git a/pytest_postgresql/factories/process.py b/pytest_postgresql/factories/process.py index f38899d4..ac3cc36b 100644 --- a/pytest_postgresql/factories/process.py +++ b/pytest_postgresql/factories/process.py @@ -227,7 +227,6 @@ def _cleanup_executor_resources() -> None: dbname=postgresql_executor.template_dbname, maintenance_dbname=postgresql_executor.maintenance_dbname, as_template=True, - version=postgresql_executor.version, password=postgresql_executor.password, autocommit=janitor_load_autocommit, ) diff --git a/pytest_postgresql/janitor.py b/pytest_postgresql/janitor.py index a454c37c..c7c7b97e 100644 --- a/pytest_postgresql/janitor.py +++ b/pytest_postgresql/janitor.py @@ -2,6 +2,7 @@ import asyncio import inspect +import warnings from contextlib import asynccontextmanager, contextmanager from pathlib import Path from types import TracebackType @@ -9,15 +10,12 @@ import psycopg import psycopg.sql as sql -from packaging.version import parse +from packaging.version import Version, parse from psycopg import AsyncCursor, Connection, Cursor from pytest_postgresql.loader import build_loader, sql_async from pytest_postgresql.retry import retry, retry_async -Version = type(parse("1")) - - DatabaseJanitorType = TypeVar("DatabaseJanitorType", bound="DatabaseJanitor") AsyncDatabaseJanitorType = TypeVar("AsyncDatabaseJanitorType", bound="AsyncDatabaseJanitor") @@ -36,7 +34,7 @@ class BaseDatabaseJanitor: _connection_timeout: int isolation_level: "psycopg.IsolationLevel | None" autocommit: bool - version: Version # type: ignore[valid-type] + version: Version | None def __init__( self, @@ -44,7 +42,7 @@ def __init__( user: str, host: str, port: str | int, - version: str | float | Version, # type: ignore[valid-type] + version: str | float | Version | None = None, dbname: str, template_dbname: str | None = None, maintenance_dbname: str = "postgres", @@ -66,7 +64,7 @@ def __init__( created or dropped from a connection to itself, so this has to be a database that already exists and that ``user`` can connect to. :param as_template: whether to mark the database as a template - :param version: postgresql version number + :param version: deprecated postgresql version number :param password: optional postgresql password :param isolation_level: optional postgresql isolation level defaults to server's default @@ -88,10 +86,16 @@ def __init__( self._connection_timeout = connection_timeout self.isolation_level = isolation_level self.autocommit = autocommit - if not isinstance(version, Version): - self.version = parse(str(version)) - else: + if version is not None: + warnings.warn( + "version argument is deprecated and will be removed in a future release", + DeprecationWarning, + stacklevel=2, + ) + if version is None or isinstance(version, Version): self.version = version + else: + self.version = parse(str(version)) def is_template(self) -> bool: """Determine whether the janitor maintains template or database.""" diff --git a/tests/test_janitor.py b/tests/test_janitor.py index 36c8d816..17cd45fe 100644 --- a/tests/test_janitor.py +++ b/tests/test_janitor.py @@ -21,24 +21,32 @@ @pytest.mark.parametrize("version", (VERSION, 10, "10")) -def test_version_cast(version: Any) -> None: - """Test that version is cast to Version object.""" - janitor = DatabaseJanitor(user="user", host="host", port="1234", dbname="database_name", version=version) +def test_version_is_deprecated(version: Any) -> None: + """Test that version is cast to Version object with a deprecation warning.""" + with pytest.warns(DeprecationWarning, match="version argument is deprecated"): + janitor = DatabaseJanitor(user="user", host="host", port="1234", dbname="database_name", version=version) assert janitor.version == VERSION +def test_version_is_optional() -> None: + """Test that janitors no longer require a PostgreSQL version.""" + janitor = DatabaseJanitor(user="user", host="host", port="1234", dbname="database_name") + assert janitor.version is None + + @pytest.mark.parametrize("version", (VERSION, 10, "10")) @pytest.mark.asyncio -async def test_version_cast_async(version: Any) -> None: - """Async test that version is cast to Version object.""" - janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="database_name", version=version) +async def test_version_is_deprecated_async(version: Any) -> None: + """Async test that version is cast to Version object with a deprecation warning.""" + with pytest.warns(DeprecationWarning, match="version argument is deprecated"): + janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="database_name", version=version) assert janitor.version == VERSION @patch("pytest_postgresql.janitor.psycopg.connect") def test_cursor_selects_postgres_database(connect_mock: MagicMock) -> None: """Test that the cursor requests the postgres database.""" - janitor = DatabaseJanitor(user="user", host="host", port="1234", dbname="database_name", version=10) + janitor = DatabaseJanitor(user="user", host="host", port="1234", dbname="database_name") with janitor.cursor(): connect_mock.assert_called_once_with(dbname="postgres", user="user", password=None, host="host", port="1234") @@ -49,7 +57,7 @@ async def test_cursor_selects_postgres_database_async() -> None: conn_mock = _make_async_conn_mock() connect_mock = AsyncMock(return_value=conn_mock) with patch("pytest_postgresql.janitor.psycopg.AsyncConnection.connect", connect_mock): - janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="database_name", version=10) + janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="database_name") async with janitor.cursor(): connect_mock.assert_called_once_with( dbname="postgres", user="user", password=None, host="host", port="1234" @@ -64,7 +72,6 @@ def test_cursor_connects_with_password(connect_mock: MagicMock) -> None: host="host", port="1234", dbname="database_name", - version=10, password=TEST_PASSWORD, ) with janitor.cursor(): @@ -84,7 +91,6 @@ async def test_cursor_connects_with_password_async() -> None: host="host", port="1234", dbname="database_name", - version=10, password=TEST_PASSWORD, ) async with janitor.cursor(): @@ -102,7 +108,6 @@ def test_cursor_selects_maintenance_database(connect_mock: MagicMock) -> None: port="1234", dbname="database_name", maintenance_dbname="maintenance_db", - version=10, ) with janitor.cursor(): connect_mock.assert_called_once_with( @@ -122,7 +127,6 @@ async def test_cursor_selects_maintenance_database_async() -> None: port="1234", dbname="database_name", maintenance_dbname="maintenance_db", - version=10, ) async with janitor.cursor(): connect_mock.assert_called_once_with( @@ -139,7 +143,6 @@ def test_cursor_dbname_overrides_maintenance_database(connect_mock: MagicMock) - port="1234", dbname="database_name", maintenance_dbname="maintenance_db", - version=10, ) with janitor.cursor(dbname="custom_db"): connect_mock.assert_called_once_with(dbname="custom_db", user="user", password=None, host="host", port="1234") @@ -151,7 +154,7 @@ async def test_cursor_custom_dbname_async() -> None: conn_mock = _make_async_conn_mock() connect_mock = AsyncMock(return_value=conn_mock) with patch("pytest_postgresql.janitor.psycopg.AsyncConnection.connect", connect_mock): - janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="database_name", version=10) + janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="database_name") async with janitor.cursor(dbname="custom_db"): connect_mock.assert_called_once_with( dbname="custom_db", user="user", password=None, host="host", port="1234" @@ -164,7 +167,7 @@ async def test_cursor_skips_isolation_level_when_none_async() -> None: conn_mock = _make_async_conn_mock() connect_mock = AsyncMock(return_value=conn_mock) with patch("pytest_postgresql.janitor.psycopg.AsyncConnection.connect", connect_mock): - janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="database_name", version=10) + janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="database_name") async with janitor.cursor(): pass @@ -188,7 +191,7 @@ def test_janitor_populate(connect_mock: MagicMock, load_database: str) -> None: "password": TEST_PASSWORD, "autocommit": False, } - janitor = DatabaseJanitor(version=10, **call_kwargs) # type: ignore[arg-type] + janitor = DatabaseJanitor(**call_kwargs) # type: ignore[arg-type] janitor.load(load_database) assert connect_mock.called assert connect_mock.call_args.kwargs == call_kwargs @@ -211,7 +214,7 @@ async def test_janitor_populate_async(connect_mock: MagicMock, load_database: st "password": TEST_PASSWORD, "autocommit": False, } - janitor = AsyncDatabaseJanitor(version=10, **call_kwargs) # type: ignore[arg-type] + janitor = AsyncDatabaseJanitor(**call_kwargs) # type: ignore[arg-type] await janitor.load(load_database) assert connect_mock.called assert connect_mock.call_args.kwargs == call_kwargs @@ -221,7 +224,6 @@ async def test_janitor_populate_async(connect_mock: MagicMock, load_database: st def test_janitor_load_forwards_autocommit(connect_mock: MagicMock) -> None: """DatabaseJanitor.load forwards the autocommit flag to the loader connection.""" janitor = DatabaseJanitor( - version=10, host="host", port="1234", user="user", @@ -238,7 +240,6 @@ def test_janitor_load_forwards_autocommit(connect_mock: MagicMock) -> None: async def test_janitor_load_forwards_autocommit_async(connect_mock: MagicMock) -> None: """AsyncDatabaseJanitor.load forwards the autocommit flag to the loader connection.""" janitor = AsyncDatabaseJanitor( - version=10, host="host", port="1234", user="user", @@ -266,7 +267,7 @@ async def test_janitor_populate_async_awaitable_loader() -> None: async def async_loader(**kwargs: object) -> None: await loader_mock(**kwargs) - janitor = AsyncDatabaseJanitor(version=10, **call_kwargs) # type: ignore[arg-type] + janitor = AsyncDatabaseJanitor(**call_kwargs) # type: ignore[arg-type] await janitor.load(async_loader) loader_mock.assert_awaited_once_with(**call_kwargs) @@ -287,7 +288,7 @@ async def test_janitor_populate_async_sync_loader_returns_awaitable() -> None: def sync_loader(**kwargs: object) -> object: return loader_mock(**kwargs) - janitor = AsyncDatabaseJanitor(version=10, **call_kwargs) # type: ignore[arg-type] + janitor = AsyncDatabaseJanitor(**call_kwargs) # type: ignore[arg-type] await janitor.load(sync_loader) loader_mock.assert_awaited_once_with(**call_kwargs) @@ -301,7 +302,6 @@ async def test_janitor_populate_async_sql_path(postgresql_proc: PostgreSQLExecut host=postgresql_proc.host, port=postgresql_proc.port, dbname=dbname, - version=postgresql_proc.version, password=postgresql_proc.password, connection_timeout=5, ) @@ -361,7 +361,6 @@ async def test_async_janitor_init_and_drop(postgresql_proc: PostgreSQLExecutor) host=postgresql_proc.host, port=postgresql_proc.port, dbname=dbname, - version=postgresql_proc.version, password=postgresql_proc.password, connection_timeout=5, ) @@ -383,7 +382,6 @@ async def test_async_janitor_template_flag_and_context_manager(postgresql_proc: host=postgresql_proc.host, port=postgresql_proc.port, dbname=dbname, - version=postgresql_proc.version, password=postgresql_proc.password, as_template=True, connection_timeout=5, @@ -403,7 +401,6 @@ async def test_async_janitor_creates_database_from_template(postgresql_proc: Pos host=postgresql_proc.host, port=postgresql_proc.port, dbname=base_dbname, - version=postgresql_proc.version, password=postgresql_proc.password, as_template=True, connection_timeout=5, @@ -414,7 +411,6 @@ async def test_async_janitor_creates_database_from_template(postgresql_proc: Pos port=postgresql_proc.port, dbname=clone_dbname, template_dbname=base_dbname, - version=postgresql_proc.version, password=postgresql_proc.password, connection_timeout=5, ) @@ -450,20 +446,20 @@ async def test_async_janitor_creates_database_from_template(postgresql_proc: Pos def test_async_janitor_is_template_false() -> None: """is_template() returns False when as_template is not set.""" - janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="mydb", version=10) + janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="mydb") assert janitor.is_template() is False def test_async_janitor_is_template_true() -> None: """is_template() returns True when as_template=True.""" - janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="mydb", as_template=True, version=10) + janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="mydb", as_template=True) assert janitor.is_template() is True @pytest.mark.asyncio async def test_async_janitor_context_manager_calls_init_and_drop() -> None: """__aenter__ calls init() and __aexit__ calls drop().""" - janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="mydb", version=10) + janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="mydb") init_mock = AsyncMock() drop_mock = AsyncMock() with patch.object(AsyncDatabaseJanitor, "init", init_mock), patch.object(AsyncDatabaseJanitor, "drop", drop_mock): @@ -488,7 +484,7 @@ async def test_async_janitor_terminate_connection_sql() -> None: @pytest.mark.asyncio async def test_async_janitor_drop_noop_when_database_missing() -> None: """drop() is a no-op when the target database does not exist.""" - janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="missing_db", version=10) + janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="missing_db") cur = AsyncMock(spec=AsyncCursor) cur.fetchone.return_value = None with patch.object(janitor, "cursor") as cursor_ctx: @@ -500,7 +496,7 @@ async def test_async_janitor_drop_noop_when_database_missing() -> None: def test_janitor_drop_noop_when_database_missing() -> None: """drop() is a no-op when the target database does not exist.""" - janitor = DatabaseJanitor(user="user", host="host", port="1234", dbname="missing_db", version=10) + janitor = DatabaseJanitor(user="user", host="host", port="1234", dbname="missing_db") cur = MagicMock() cur.fetchone.return_value = None with patch.object(janitor, "cursor") as cursor_ctx: @@ -513,7 +509,7 @@ def test_janitor_drop_noop_when_database_missing() -> None: @pytest.mark.asyncio async def test_async_janitor_load_sql_path_raises_without_aiofiles() -> None: """AsyncDatabaseJanitor.load() surfaces aiofiles ImportError for SQL file paths.""" - janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="mydb", version=10) + janitor = AsyncDatabaseJanitor(user="user", host="host", port="1234", dbname="mydb") with patch("pytest_postgresql.loader.aiofiles", None): with pytest.raises(ImportError, match="aiofiles"): await janitor.load(Path("dummy.sql")) diff --git a/tests/test_noopexecutor.py b/tests/test_noopexecutor.py index a8108b44..eb73e3e9 100644 --- a/tests/test_noopexecutor.py +++ b/tests/test_noopexecutor.py @@ -56,7 +56,6 @@ def test_noproc_version_uses_maintenance_dbname(postgresql_proc: PostgreSQLExecu host=postgresql_proc.host, port=postgresql_proc.port, dbname="maintenance_for_version", - version=postgresql_proc.version, password=postgresql_proc.password, ): postgresql_noproc = NoopExecutor( diff --git a/tests/test_postgres_options_plugin.py b/tests/test_postgres_options_plugin.py index 47f5a4f5..a96441be 100644 --- a/tests/test_postgres_options_plugin.py +++ b/tests/test_postgres_options_plugin.py @@ -100,7 +100,6 @@ def maintenance_dbname(postgresql_proc_to_override: PostgreSQLExecutor) -> Itera host=postgresql_proc_to_override.host, port=postgresql_proc_to_override.port, dbname=dbname, - version=postgresql_proc_to_override.version, password=postgresql_proc_to_override.password, connection_timeout=5, ): @@ -176,7 +175,6 @@ def _run_drop_test_database_case( port=postgresql_proc_to_override.port, dbname=template_dbname, as_template=True, - version=postgresql_proc_to_override.version, password=postgresql_proc_to_override.password, connection_timeout=5, ) @@ -189,7 +187,6 @@ def _run_drop_test_database_case( port=postgresql_proc_to_override.port, dbname=dbname, template_dbname=template_janitor.dbname, - version=postgresql_proc_to_override.version, password=postgresql_proc_to_override.password, connection_timeout=5, )