Move psql CLI orchestration off DatabaseConnection

runshell, executable_name, and the psql argv/env builder were CLI-layer
concerns bolted onto DatabaseConnection for Django-historical reasons —
they never used the psycopg connection, only read settings_dict. Moving
them out lets DatabaseConnection drop three stdlib imports (os, signal,
subprocess) and shrink toward its actual job (transactions, cursors,
lifecycle).

Extract postgres_cli_args() and postgres_cli_env() as public helpers
in plain.postgres.database_url. The plain postgres shell command
composes them inline with its SIGINT dance. PostgresBackupClient in
plain-dev had ~35 lines of the same DatabaseConfig → argv/env mapping
duplicated; it now calls the shared helpers.

Author: davegaeddert

plain-dev/plain/dev/backups/clients.py

@@ -6,6 +6,7 @@
 from typing import TYPE_CHECKING
 
 from plain.exceptions import ImproperlyConfigured
+from plain.postgres.database_url import postgres_cli_args, postgres_cli_env
 
 if TYPE_CHECKING:
     from plain.postgres.connection import DatabaseConnection
@@ -15,52 +16,17 @@ class PostgresBackupClient:
     def __init__(self, connection: DatabaseConnection) -> None:
         self.connection = connection
 
-    def get_env(self) -> dict[str, str]:
-        settings_dict = self.connection.settings_dict
-        options = settings_dict.get("OPTIONS", {})
-        env: dict[str, str] = {}
-
-        if password := settings_dict.get("PASSWORD"):
-            env["PGPASSWORD"] = str(password)
-
-        # Map OPTIONS keys to their corresponding environment variables.
-        option_env_vars = {
-            "passfile": "PGPASSFILE",
-            "sslmode": "PGSSLMODE",
-            "sslrootcert": "PGSSLROOTCERT",
-            "sslcert": "PGSSLCERT",
-            "sslkey": "PGSSLKEY",
-        }
-        for option_key, env_var in option_env_vars.items():
-            if value := options.get(option_key):
-                env[env_var] = str(value)
-
-        return env
-
-    def _get_conn_args(self) -> list[str]:
-        """Build common connection CLI args from settings."""
-        settings_dict = self.connection.settings_dict
-        args: list[str] = []
-        if user := settings_dict.get("USER"):
-            args += ["-U", user]
-        if host := settings_dict.get("HOST"):
-            args += ["-h", host]
-        if port := settings_dict.get("PORT"):
-            args += ["-p", str(port)]
-        return args
-
     def _run(self, cmd: str | list[str], *, shell: bool = False) -> None:
-        subprocess.run(
-            cmd, env={**os.environ, **self.get_env()}, check=True, shell=shell
-        )
+        env = {**os.environ, **postgres_cli_env(self.connection.settings_dict)}
+        subprocess.run(cmd, env=env, check=True, shell=shell)
 
     def create_backup(self, backup_path: Path, *, pg_dump: str = "pg_dump") -> None:
         settings_dict = self.connection.settings_dict
         dbname = settings_dict.get("DATABASE")
         if not dbname:
             raise ImproperlyConfigured("POSTGRES_DATABASE is required in settings")
 
-        args = pg_dump.split() + self._get_conn_args()
+        args = pg_dump.split() + postgres_cli_args(settings_dict)
         args += ["-Fc", dbname]
 
         # Pipe through gzip for compression
@@ -75,7 +41,7 @@ def restore_backup(
         if not dbname:
             raise ImproperlyConfigured("POSTGRES_DATABASE is required in settings")
 
-        conn_args = self._get_conn_args()
+        conn_args = postgres_cli_args(settings_dict)
 
         # Drop and recreate the database via template1
         drop_create_cmds = [

plain-postgres/plain/postgres/cli/core.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import os
+import signal
 import subprocess
 import sys
 import time
@@ -10,6 +12,7 @@
 
 from plain.cli import register_cli
 
+from ..database_url import postgres_cli_args, postgres_cli_env
 from ..db import get_connection
 from ..dialect import quote_name
 from .converge import converge
@@ -36,16 +39,20 @@ def cli() -> None:
 @database_management_command
 def shell(parameters: tuple[str, ...]) -> None:
     """Open an interactive database shell"""
-    conn = get_connection()
+    config = get_connection().settings_dict
+    args = ["psql", *postgres_cli_args(config), *parameters, config["DATABASE"]]
+    env = {**os.environ, **postgres_cli_env(config)}
+    sigint_handler = signal.getsignal(signal.SIGINT)
     try:
-        conn.runshell(list(parameters))
+        # Allow SIGINT to pass to psql to abort queries.
+        signal.signal(signal.SIGINT, signal.SIG_IGN)
+        subprocess.run(args, env=env, check=True)
     except FileNotFoundError:
-        # Note that we're assuming the FileNotFoundError relates to the
-        # command missing. It could be raised for some other reason, in
-        # which case this error message would be inaccurate. Still, this
-        # message catches the common case.
+        # FileNotFoundError almost always means psql isn't installed or on
+        # PATH, but could be raised for other reasons — the message covers
+        # the common case.
         click.secho(
-            f"You appear not to have the {conn.executable_name!r} program installed or on your path.",
+            "You appear not to have the 'psql' program installed or on your path.",
             fg="red",
             err=True,
         )
@@ -60,6 +67,8 @@ def shell(parameters: tuple[str, ...]) -> None:
             err=True,
         )
         sys.exit(e.returncode)
+    finally:
+        signal.signal(signal.SIGINT, sigint_handler)
 
 
 @cli.command("drop-unknown-tables")

plain-postgres/plain/postgres/connection.py

@@ -1,9 +1,6 @@
 from __future__ import annotations
 
 import _thread
-import os
-import signal
-import subprocess
 import warnings
 from collections import deque
 from collections.abc import Generator, Sequence
@@ -57,41 +54,6 @@ class TableInfo(NamedTuple):
     comment: str | None
 
 
-def _psql_settings_to_cmd_args_env(
-    settings_dict: DatabaseConfig, parameters: list[str]
-) -> tuple[list[str], dict[str, str] | None]:
-    """Build psql command-line arguments from database settings."""
-    args = ["psql"]
-    options = settings_dict.get("OPTIONS", {})
-
-    if user := settings_dict.get("USER"):
-        args += ["-U", user]
-    if host := settings_dict.get("HOST"):
-        args += ["-h", host]
-    if port := settings_dict.get("PORT"):
-        args += ["-p", str(port)]
-    args.extend(parameters)
-    args += [settings_dict["DATABASE"]]
-
-    env: dict[str, str] = {}
-    if password := settings_dict.get("PASSWORD"):
-        env["PGPASSWORD"] = str(password)
-
-    # Map OPTIONS keys to their corresponding environment variables.
-    option_env_vars = {
-        "passfile": "PGPASSFILE",
-        "sslmode": "PGSSLMODE",
-        "sslrootcert": "PGSSLROOTCERT",
-        "sslcert": "PGSSLCERT",
-        "sslkey": "PGSSLKEY",
-    }
-    for option_key, env_var in option_env_vars.items():
-        if value := options.get(option_key):
-            env[env_var] = str(value)
-
-    return args, (env or None)
-
-
 class DatabaseConnection:
     """
     PostgreSQL database connection.
@@ -100,7 +62,6 @@ class DatabaseConnection:
     """
 
     queries_limit: int = 9000
-    executable_name: str = "psql"
 
     index_default_access_method = "btree"
     ignored_tables: list[str] = []
@@ -415,19 +376,6 @@ def schema_editor(self, *args: Any, **kwargs: Any) -> DatabaseSchemaEditor:
         """Return a new instance of the schema editor."""
         return DatabaseSchemaEditor(self, *args, **kwargs)
 
-    def runshell(self, parameters: list[str]) -> None:
-        """Run an interactive psql shell."""
-        args, env = _psql_settings_to_cmd_args_env(self.settings_dict, parameters)
-        env = {**os.environ, **env} if env else None
-        sigint_handler = signal.getsignal(signal.SIGINT)
-        try:
-            # Allow SIGINT to pass to psql to abort queries.
-            signal.signal(signal.SIGINT, signal.SIG_IGN)
-            subprocess.run(args, env=env, check=True)
-        finally:
-            # Restore the original SIGINT handler.
-            signal.signal(signal.SIGINT, sigint_handler)
-
     def on_commit(self, func: Any, robust: bool = False) -> None:
         if not callable(func):
             raise TypeError("on_commit()'s callback must be a callable.")

plain-postgres/plain/postgres/database_url.py

@@ -77,6 +77,37 @@ def parse_database_url(url: str) -> DatabaseConfig:
     }
 
 
+_CLI_FLAGS: list[tuple[str, str]] = [("USER", "-U"), ("HOST", "-h"), ("PORT", "-p")]
+_CLI_OPTION_ENV_VARS: dict[str, str] = {
+    "passfile": "PGPASSFILE",
+    "sslmode": "PGSSLMODE",
+    "sslrootcert": "PGSSLROOTCERT",
+    "sslcert": "PGSSLCERT",
+    "sslkey": "PGSSLKEY",
+}
+
+
+def postgres_cli_args(config: DatabaseConfig) -> list[str]:
+    """Build connection flags for libpq-based tools (psql, pg_dump, pg_restore)."""
+    args: list[str] = []
+    for key, flag in _CLI_FLAGS:
+        if value := config.get(key):
+            args += [flag, str(value)]
+    return args
+
+
+def postgres_cli_env(config: DatabaseConfig) -> dict[str, str]:
+    """Build env vars for libpq-based tools (psql, pg_dump, pg_restore)."""
+    env: dict[str, str] = {}
+    if password := config.get("PASSWORD"):
+        env["PGPASSWORD"] = str(password)
+    options = config.get("OPTIONS", {})
+    for option_key, env_var in _CLI_OPTION_ENV_VARS.items():
+        if value := options.get(option_key):
+            env[env_var] = str(value)
+    return env
+
+
 def build_database_url(config: DatabaseConfig) -> str:
     """Build a database URL from a configuration dictionary."""
     options = config.get("OPTIONS", {})