Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 19 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,19 @@ Defining pre-population on the command line:

pytest --postgresql-load=path/to/file.sql --postgresql-load=path.to.function

If a loaded ``.sql`` file contains statements that cannot run inside a transaction
block (for example ``CREATE DATABASE``), enable autocommit on the loader connection.
This can be set without writing a custom loader, via the factory argument, the command
line, or ``pytest.ini``:

.. code-block:: python

postgresql_my_proc = factories.postgresql_proc(load=[Path("with_create_db.sql")], load_autocommit=True)

.. code-block:: sh

pytest --postgresql-load-autocommit

Connecting to an existing PostgreSQL database
----------------------------------------------

Expand Down Expand Up @@ -370,6 +383,12 @@ You can define settings via fixture factory arguments, command line options, or
- postgresql_load
- yes
-
* - Autocommit for the SQL loader connection
- load_autocommit
- --postgresql-load-autocommit
- postgresql_load_autocommit
- yes
- False
Comment thread
coderabbitai[bot] marked this conversation as resolved.
* - PostgreSQL connection options
- options
- --postgresql-options
Expand Down
1 change: 1 addition & 0 deletions newsfragments/902.break.1.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Load functions passed to proc/noproc fixtures have to accept new `autocommit` argument.
2 changes: 2 additions & 0 deletions newsfragments/902.break.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
`postgresql_noproc` has an additional parameter `load_autocommit` which is grouped together with `load` making
positional arguments filled till `depends_on` incompatible.
1 change: 1 addition & 0 deletions newsfragments/902.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow running the SQL loader connection with autocommit enabled, so ``.sql`` files containing statements that cannot run inside a transaction block (such as ``CREATE DATABASE``) can be loaded. This is configurable without a custom loader in three ways: the ``postgresql_load_autocommit`` ini option, the ``--postgresql-load-autocommit`` command line flag, or the ``load_autocommit`` argument of the ``postgresql_proc`` / ``postgresql_noproc`` factories (which also maps to an ``autocommit`` argument on ``DatabaseJanitor`` / ``AsyncDatabaseJanitor``).
2 changes: 2 additions & 0 deletions pytest_postgresql/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ class PostgreSQLConfig:
unixsocketdir: str
dbname: str
load: list[Path | str]
load_autocommit: bool
postgres_options: str
drop_test_database: bool

Expand All @@ -49,6 +50,7 @@ def get_postgresql_option(option: str) -> Any:
unixsocketdir=get_postgresql_option("unixsocketdir"),
dbname=get_postgresql_option("dbname"),
load=load_paths,
load_autocommit=bool(get_postgresql_option("load_autocommit")),
postgres_options=get_postgresql_option("postgres_options"),
drop_test_database=request.config.getoption("postgresql_drop_test_database"),
)
Expand Down
9 changes: 9 additions & 0 deletions pytest_postgresql/factories/noprocess.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def postgresql_noproc(
dbname: str | None = None,
options: str = "",
load: list[Callable | str | Path] | None = None,
load_autocommit: bool | None = None,
depends_on: str | None = None,
) -> Callable[[FixtureRequest], Iterator[NoopExecutor]]:
"""Postgresql noprocess factory.
Expand All @@ -56,6 +57,9 @@ def postgresql_noproc(
:param dbname: postgresql database name
:param options: Postgresql connection options
:param load: List of functions used to initialize database's template.
:param load_autocommit: run the SQL loader connection with autocommit on.
Required for statements that cannot run inside a transaction block,
e.g. ``CREATE DATABASE`` in a loaded ``.sql`` file.
:param depends_on: Optional name of the fixture to depend on.
:returns: function which makes a postgresql process
"""
Expand Down Expand Up @@ -96,6 +100,10 @@ def postgresql_noproc_fixture(request: FixtureRequest) -> Iterator[NoopExecutor]
else:
noop_exec_dbname = pg_dbname

janitor_load_autocommit = config.load_autocommit
if load_autocommit is not None:
janitor_load_autocommit = load_autocommit

noop_exec = NoopExecutor(
host=pg_host,
port=pg_port,
Expand All @@ -113,6 +121,7 @@ def postgresql_noproc_fixture(request: FixtureRequest) -> Iterator[NoopExecutor]
as_template=True,
version=noop_exec.version,
password=noop_exec.password,
autocommit=janitor_load_autocommit,
)
if drop_test_database:
janitor.drop()
Expand Down
8 changes: 8 additions & 0 deletions pytest_postgresql/factories/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ def postgresql_proc(
unixsocketdir: str | None = None,
postgres_options: str | None = None,
load: list[Callable | str | Path] | None = None,
load_autocommit: bool | None = None,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
) -> Callable[[FixtureRequest, TempPathFactory], PostgreSQLExecutor]:
"""Postgresql process factory.

Expand All @@ -114,6 +115,9 @@ def postgresql_proc(
:param unixsocketdir: directory to create postgresql's unixsockets
:param postgres_options: Postgres executable options for use by pg_ctl
:param load: List of functions used to initialize database's template.
:param load_autocommit: run the SQL loader connection with autocommit on.
Required for statements that cannot run inside a transaction block,
e.g. ``CREATE DATABASE`` in a loaded ``.sql`` file.
:returns: function which makes a postgresql process
"""

Expand Down Expand Up @@ -212,6 +216,9 @@ def _cleanup_executor_resources() -> None:
)
postgresql_executor.start()
postgresql_executor.wait_for_postgres()
janitor_load_autocommit = config.load_autocommit
if load_autocommit is not None:
janitor_load_autocommit = load_autocommit
janitor = DatabaseJanitor(
user=postgresql_executor.user,
host=postgresql_executor.host,
Expand All @@ -220,6 +227,7 @@ def _cleanup_executor_resources() -> None:
as_template=True,
version=postgresql_executor.version,
password=postgresql_executor.password,
autocommit=janitor_load_autocommit,
)
if config.drop_test_database:
janitor.drop()
Expand Down
9 changes: 9 additions & 0 deletions pytest_postgresql/janitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class BaseDatabaseJanitor:
as_template: bool
_connection_timeout: int
isolation_level: "psycopg.IsolationLevel | None"
autocommit: bool
version: Version # type: ignore[valid-type]

def __init__(
Expand All @@ -48,6 +49,7 @@ def __init__(
as_template: bool = False,
password: str | None = None,
isolation_level: "psycopg.IsolationLevel | None" = None,
autocommit: bool = False,
connection_timeout: int = 60,
) -> None:
"""Initialize janitor.
Expand All @@ -62,6 +64,10 @@ def __init__(
:param password: optional postgresql password
:param isolation_level: optional postgresql isolation level
defaults to server's default
:param autocommit: run the SQL loader connection with autocommit on.
Required for statements that cannot run inside a transaction
block, e.g. ``CREATE DATABASE`` in a loaded ``.sql`` file. Only
affects the loader connection, not the janitor's admin cursor.
:param connection_timeout: how long to retry connection before
raising a TimeoutError
"""
Expand All @@ -74,6 +80,7 @@ def __init__(
self.as_template = as_template
self._connection_timeout = connection_timeout
self.isolation_level = isolation_level
self.autocommit = autocommit
if not isinstance(version, Version):
self.version = parse(str(version))
else:
Expand Down Expand Up @@ -154,6 +161,7 @@ def load(self, load: Callable | str | Path) -> None:
user=self.user,
dbname=self.dbname,
password=self.password,
autocommit=self.autocommit,
)

@contextmanager
Expand Down Expand Up @@ -259,6 +267,7 @@ async def load(self, load: Callable | str | Path) -> None:
"user": self.user,
"dbname": self.dbname,
"password": self.password,
"autocommit": self.autocommit,
}
loader_func = getattr(_loader, "func", _loader)
if inspect.iscoroutinefunction(loader_func):
Expand Down
13 changes: 13 additions & 0 deletions pytest_postgresql/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@
"when database was not cleared due to errors in previous test runs. "
"Use cautiously and not on CI."
)
_help_load_autocommit = (
"Run the SQL loader connection with autocommit enabled. "
"Required for statements that cannot run inside a transaction block, "
"e.g. CREATE DATABASE in a loaded .sql file."
)

# psycopg async cannot use Windows' default ProactorEventLoop (the default since
# Python 3.8). libpq socket I/O relies on selector APIs (add_reader / fileno)
Expand Down Expand Up @@ -163,6 +168,7 @@ def pytest_addoption(parser: Parser) -> None:

parser.addini(name="postgresql_load", type="pathlist", help=_help_load)
parser.addini(name="postgresql_postgres_options", help=_help_postgres_options, default="")
parser.addini(name="postgresql_load_autocommit", type="bool", default=False, help=_help_load_autocommit)

parser.addoption(
"--postgresql-exec",
Expand Down Expand Up @@ -225,6 +231,13 @@ def pytest_addoption(parser: Parser) -> None:
help=_help_drop_test_database,
)

parser.addoption(
"--postgresql-load-autocommit",
action="store_true",
dest="postgresql_load_autocommit",
help=_help_load_autocommit,
)


postgresql_proc = factories.postgresql_proc()
postgresql_noproc = factories.postgresql_noproc()
Expand Down
14 changes: 14 additions & 0 deletions tests/examples/test_assert_load_autocommit_is_true.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
"""Asserts that load_autocommit is True.

That shows that it is not the default (False), and that we parsed it as a bool.
"""

from pytest import FixtureRequest

from pytest_postgresql.config import get_config


def test_assert_load_autocommit_is_true(request: FixtureRequest) -> None:
"""Asserts that load_autocommit is True."""
config = get_config(request)
assert config.load_autocommit is True
9 changes: 6 additions & 3 deletions tests/test_chaining.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,29 @@
from pytest_postgresql.executor_noop import NoopExecutor


def load_schema(host: str, port: int, user: str, dbname: str, password: str | None) -> None:
def load_schema(host: str, port: int, user: str, dbname: str, password: str | None, autocommit: bool) -> None:

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

Make autocommit keyword-only.

Ruff flags each new boolean positional parameter (FBT001). The janitor already passes loader arguments by keyword, so this is a safe API clarification:

-def load_schema(host: str, port: int, user: str, dbname: str, password: str | None, autocommit: bool) -> None:
+def load_schema(host: str, port: int, user: str, dbname: str, password: str | None, *, autocommit: bool) -> None:

Apply the same change to load_data and load_more_data.

Also applies to: 19-19, 29-29

🧰 Tools
🪛 Ruff (0.15.21)

[warning] 10-10: Boolean-typed positional argument in function definition

(FBT001)

🤖 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_chaining.py` at line 10, Update the signatures of load_schema,
load_data, and load_more_data so autocommit is keyword-only by placing it after
the keyword-only separator; preserve existing parameter types and behavior,
since callers already pass this argument by keyword.

Source: Linters/SAST tools

"""Load schema into the database."""
with psycopg.connect(host=host, port=port, user=user, dbname=dbname, password=password) as conn:
conn.autocommit = autocommit
with conn.cursor() as cur:
cur.execute("CREATE TABLE schema_table (id serial PRIMARY KEY, name varchar);")
conn.commit()


def load_data(host: str, port: int, user: str, dbname: str, password: str | None) -> None:
def load_data(host: str, port: int, user: str, dbname: str, password: str | None, autocommit: bool) -> None:
"""Load the first layer of data into the database."""
with psycopg.connect(host=host, port=port, user=user, dbname=dbname, password=password) as conn:
conn.autocommit = autocommit
with conn.cursor() as cur:
cur.execute("INSERT INTO schema_table (name) VALUES ('data_layer');")
cur.execute("CREATE TABLE data_table (id serial PRIMARY KEY, val varchar);")
conn.commit()


def load_more_data(host: str, port: int, user: str, dbname: str, password: str | None) -> None:
def load_more_data(host: str, port: int, user: str, dbname: str, password: str | None, autocommit: bool) -> None:
"""Load the second layer of data into the database."""
with psycopg.connect(host=host, port=port, user=user, dbname=dbname, password=password) as conn:
conn.autocommit = autocommit
with conn.cursor() as cur:
cur.execute("INSERT INTO schema_table (name) VALUES ('more_data_layer');")
cur.execute("CREATE TABLE more_data_table (id serial PRIMARY KEY, extra varchar);")
Expand Down
37 changes: 37 additions & 0 deletions tests/test_janitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ def test_janitor_populate(connect_mock: MagicMock, load_database: str) -> None:
"user": "user",
"dbname": "database_name",
"password": TEST_PASSWORD,
"autocommit": False,
}
janitor = DatabaseJanitor(version=10, **call_kwargs) # type: ignore[arg-type]
janitor.load(load_database)
Expand All @@ -156,13 +157,47 @@ async def test_janitor_populate_async(connect_mock: MagicMock, load_database: st
"user": "user",
"dbname": "database_name",
"password": TEST_PASSWORD,
"autocommit": False,
}
janitor = AsyncDatabaseJanitor(version=10, **call_kwargs) # type: ignore[arg-type]
await janitor.load(load_database)
assert connect_mock.called
assert connect_mock.call_args.kwargs == call_kwargs


@patch("pytest_postgresql.janitor.psycopg.connect")
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",
dbname="database_name",
password=TEST_PASSWORD,
autocommit=True,
)
janitor.load("tests.loader.load_database")
assert connect_mock.call_args.kwargs["autocommit"] is True


@pytest.mark.asyncio
@patch("tests.loader.psycopg.connect")
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",
dbname="database_name",
password=TEST_PASSWORD,
autocommit=True,
)
await janitor.load("tests.loader.load_database")
assert connect_mock.call_args.kwargs["autocommit"] is True


@pytest.mark.asyncio
async def test_janitor_populate_async_awaitable_loader() -> None:
"""AsyncDatabaseJanitor.load awaits async loader callables."""
Expand All @@ -172,6 +207,7 @@ async def test_janitor_populate_async_awaitable_loader() -> None:
"user": "user",
"dbname": "database_name",
"password": TEST_PASSWORD,
"autocommit": False,
}
loader_mock = AsyncMock()

Expand All @@ -192,6 +228,7 @@ async def test_janitor_populate_async_sync_loader_returns_awaitable() -> None:
"user": "user",
"dbname": "database_name",
"password": TEST_PASSWORD,
"autocommit": False,
}
loader_mock = AsyncMock()

Expand Down
15 changes: 15 additions & 0 deletions tests/test_postgres_options_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,21 @@ def test_postgres_loader_in_ini(pointed_pytester: Pytester) -> None:
ret.assert_outcomes(passed=1)


def test_postgres_load_autocommit_in_cli(pointed_pytester: Pytester) -> None:
"""Check that the --postgresql-load-autocommit command line flag is honored."""
pointed_pytester.copy_example("test_assert_load_autocommit_is_true.py")
ret = pointed_pytester.runpytest("--postgresql-load-autocommit", "test_assert_load_autocommit_is_true.py")
ret.assert_outcomes(passed=1)


def test_postgres_load_autocommit_in_ini(pointed_pytester: Pytester) -> None:
"""Check that pytest.ini postgresql_load_autocommit is honored and parsed as a bool."""
pointed_pytester.copy_example("test_assert_load_autocommit_is_true.py")
pointed_pytester.makefile(".ini", pytest="[pytest]\npostgresql_load_autocommit = True\n")
ret = pointed_pytester.runpytest("test_assert_load_autocommit_is_true.py")
ret.assert_outcomes(passed=1)


def test_postgres_port_search_count_in_cli_is_int(pointed_pytester: Pytester) -> None:
"""Check that the --postgresql-port-search-count command line argument is parsed as an int."""
pointed_pytester.copy_example("test_assert_port_search_count_is_ten.py")
Expand Down