Fix pgAdmin fails when performing Backup/Restore on a PostgreSQL connection defined exclusively via pg_service.conf. #9553

[RohitBhati8269]

Detail Info: #9553

Summary by CodeRabbit

• New Features

  • Backup and restore now respect explicitly provided command arguments for host/port, enabling argument-driven overrides.

• Bug Fixes / Improvements

  • Command arguments for backup/restore are assembled dynamically and include connection parameters only when present.
  • Server name display now includes optional service, host and port when available and omits empty fields.
  • Improved resilience to missing host, port, or username values during operations.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds an arg_list attribute to BackupMessage and RestoreMessage; details() stores incoming args. Argument lists for backup/restore/sql utilities are built incrementally from arg_list, SSH-tunnel bindings, or server defaults. get_server_name prefers arg-derived host/port and formats service/host/port, omitting absent fields.

Changes

Cohort / File(s)	Summary
Backup argument construction web/pgadmin/tools/backup/__init__.py	Adds arg_list to BackupMessage; details() stores incoming args. _get_args_params_values now starts with --file and conditionally appends --host, --port (skips 'None'), --username, always appends --no-password. get_server_name reads host/port from arg_list when present, otherwise uses SSH-tunnel or server values; output omits empty fields.
Restore argument and server name handling web/pgadmin/tools/restore/__init__.py	Adds arg_list to RestoreMessage; details() stores incoming args. get_server_name prefers arg_list host/port and includes optional Service label. get_restore_util_args and get_sql_util_args build args incrementally, conditionally adding --host, --port, --username only when present (preserving SSH-tunnel/server fallbacks) and always appending --no-password.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Message as Backup/RestoreMessage
    participant SSH as SSHTunnelManager
    participant Server
    participant Util as pg_dump/pg_restore
    Client->>Message: details(cmd, args)
    Message->>Message: store arg_list = args
    Message->>Message: get_server_name() checks arg_list
    alt host/port in arg_list
        Message->>Message: use host/port from arg_list
    else
        Message->>SSH: query binding for server_id
        alt SSH has binding
            SSH->>Message: bound_host/bound_port
        else
            Message->>Server: fetch host/port
            Server->>Message: host/port
        end
    end
    Message->>Message: build args list incrementally (--file, optional --host/--port/--username, --no-password)
    Message->>Util: execute with constructed args

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and specifically describes the primary fix: addressing pgAdmin failures during Backup/Restore operations on PostgreSQL connections defined via pg_service.conf, matching the code changes in both backup and restore modules.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@web/pgadmin/tools/restore/__init__.py`:
- Around line 368-384: The args list built in get_sql_util_args must include the
'--no-password' flag to mirror get_restore_util_args and prevent psql from
hanging waiting for interactive password input; update the args construction in
get_sql_util_args (the list containing '--dbname', '-c', '--file' etc.) to
append '--no-password' (unconditionally or under the same conditions used for
get_restore_util_args) so psql runs non-interactively in batch processes.

🧹 Nitpick comments (1)

web/pgadmin/tools/backup/__init__.py (1)

210-211: Consider documenting the 'None' string check.

The check port != 'None' guards against the case where str(server.port) yields the literal string 'None' when the port is None. While this works, it's a subtle edge case that may confuse future maintainers.

💡 Optional: Add a brief comment or refactor for clarity

-    if port and port != 'None':
+    # port may be string 'None' when server.port is None
+    if port and port != 'None':
         args.extend(['--port', port])

Alternatively, handle the conversion earlier:

port = str(manager.local_bind_port) if manager.use_ssh_tunnel else (
    str(server.port) if server.port is not None else None
)
# ...
if port:
    args.extend(['--port', port])

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@web/pgadmin/tools/backup/__init__.py`:
- Around line 138-142: The f-string is broken by nested single quotes around
gettext('Service') inside the returned f-string; fix by avoiding nested
quoting—either change the inner f-string to use double quotes (e.g.,
f"{gettext('Service')}: {s.service}") or precompute the label (label =
gettext('Service')) and use that in the return expression that references
s.name, s.service, host, port so the f-string becomes well-formed.

In `@web/pgadmin/tools/restore/__init__.py`:
- Around line 114-118: The f-string in the return expression that builds the
server name (the expression starting with return f"{s.name} (...") contains
nested single quotes around 'Service' which breaks parsing; change how the
Service label is composed—either use double quotes inside the inner f-string
(e.g., f"Service") or build the label outside the f-string and include that
variable in the join—so that the fragment f'{_('Service')}: {s.service}' is
replaced by a valid string construction (for example using _("Service") with
double quotes or precomputing service_label) and then return the final formatted
string; update the expression that references s.name, _('Service'), s.service,
host, and port accordingly.