Skip to content

Deprecate DatabaseJanitor version argument #1405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
w3lld1 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Deprecate DatabaseJanitor version argument #1405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions newsfragments/1393.depr.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deprecated the unused ``version`` argument of ``DatabaseJanitor`` and ``AsyncDatabaseJanitor``.
2 changes: 0 additions & 2 deletions pytest_postgresql/factories/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand Down
1 change: 0 additions & 1 deletion pytest_postgresql/factories/noprocess.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
1 change: 0 additions & 1 deletion pytest_postgresql/factories/process.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
24 changes: 14 additions & 10 deletions pytest_postgresql/janitor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,20 @@

import asyncio
import inspect
import warnings
from contextlib import asynccontextmanager, contextmanager
from pathlib import Path
from types import TracebackType
from typing import AsyncIterator, Callable, Iterator, Type, TypeVar

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")

Expand All @@ -36,15 +34,15 @@ class BaseDatabaseJanitor:
_connection_timeout: int
isolation_level: "psycopg.IsolationLevel | None"
autocommit: bool
version: Version # type: ignore[valid-type]
version: Version | None

def __init__(
self,
*,
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",
Expand All @@ -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
Expand All @@ -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."""
Expand Down
60 changes: 28 additions & 32 deletions tests/test_janitor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Comment on lines +24 to +27

@coderabbitai coderabbitai Bot Aug 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Replace Any with the supported version input type.

Ruff reports ANN401 for version in both deprecation tests. Use a concrete union that matches the parametrised values and BaseDatabaseJanitor.__init__, such as changing 10 to 10.0 and annotating str | float | Version. This preserves static checking.

Also applies to: 39-42

🧰 Tools
🪛 Ruff (0.16.1)

[warning] 24-24: Dynamically typed expressions (typing.Any) are disallowed in version

(ANN401)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_janitor.py` around lines 24 - 27, Replace the Any annotation on
version in both deprecation tests with str | float | Version, and change the
parametrized numeric value 10 to 10.0 so it matches the supported input type and
BaseDatabaseJanitor.__init__ contract.

Source: Linters/SAST tools

@fizyk fizyk Aug 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai file an issue to enable, it's not enabled at the moment

@coderabbitai coderabbitai Bot Aug 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="dbfixtures/pytest-postgresql"
title="Enable Ruff ANN401 checks"
body=$(cat <<'EOF'
## Summary

Enable Ruff rule `ANN401` in the repository configuration.

## Rationale

The rule detected `typing.Any` in parameter annotations in `tests/test_janitor.py`. The check is not currently enabled, so it cannot prevent similar annotations in future changes.

## Affected areas

- Ruff configuration
- Python type annotations, including `tests/test_janitor.py`

## Required changes

Enable `ANN401`, or the relevant Ruff annotation rule set that includes it, in the project lint configuration. Confirm that the configured checks run in the standard lint workflow.

## Acceptance criteria

- Ruff reports `ANN401` for parameters annotated as `typing.Any`.
- The repository lint configuration enables the rule.
- The standard lint workflow executes the enabled rule.

## Backlinks

- Pull request: https://github.com/dbfixtures/pytest-postgresql/pull/1405
- Review comment: https://github.com/dbfixtures/pytest-postgresql/pull/1405#discussion_r3748701045
- Requested by: `@fizyk`
EOF
)

gh issue create \
  --repo "$repo" \
  --title "$title" \
  --body "$body" \
  --assignee fizyk

Length of output: 2743

@fizyk, I created and assigned #1406: Enable Ruff ANN401 checks.

🧬 Code Graph Analysis Results

pytest_postgresql/janitor.py

  • Lines 222–337 — Complete AsyncDatabaseJanitor implementation, including init(), drop(), template handling, SQL loading, async cursor management, and async context-manager lifecycle methods.
  • Lines 240–253 — AsyncDatabaseJanitor.drop() checks database existence, disables connections, terminates active connections, clears the template flag when needed, and drops the database.
  • Lines 298–323 — AsyncDatabaseJanitor.cursor() connects to the configured maintenance database, applies isolation/autocommit settings, yields an async cursor, and closes the connection during cleanup.
  • Lines 268–295 — AsyncDatabaseJanitor.load() builds the loader with sql_async, forwards connection parameters and autocommit, and handles synchronous, asynchronous, and awaitable-returning loaders.

pytest_postgresql/factories/process.py

  • Lines 85–244 — postgresql_proc() fixture factory creates and starts a PostgreSQL executor, configures DatabaseJanitor, initializes the template database, loads configured data, and performs cleanup.
  • Lines 198–214 — The fixture passes load_autocommit configuration or override to DatabaseJanitor(autocommit=...), then calls janitor.drop(), janitor.init(), and janitor.load(...).

You are interacting with an AI system.

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"))
Comment thread
coderabbitai[bot] marked this conversation as resolved.
@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")

Expand All @@ -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"
Expand All @@ -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():
Expand All @@ -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():
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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")
Expand All @@ -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"
Expand All @@ -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

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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,
)
Expand Down Expand Up @@ -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,
)
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
)
Expand Down Expand Up @@ -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):
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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"))
Expand Down
1 change: 0 additions & 1 deletion tests/test_noopexecutor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading
Loading