Skip to content
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.

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

Add --ping as a CLI option #1471

Merged
merged 6 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ Contributors:
* Antonio Aguilar (crazybolillo)
* Andrew M. MacFie (amacfie)
* saucoide
* Chris Rose (offbyone/offby1)

Creator:
--------
Expand Down
7 changes: 7 additions & 0 deletions changelog.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
Dev
===

Features
--------
* Add a `--ping` command line option; allows pgcli to replace `pg_isready`

4.1.0 (2024-03-09)
==================

Expand Down
31 changes: 29 additions & 2 deletions pgcli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1463,6 +1463,13 @@
is_flag=True,
help="list available databases, then exit.",
)
@click.option(
"--ping",
"ping_database",
is_flag=True,
default=False,
help="Check database connectivity, then exit.",
)
@click.option(
"--auto-vertical-output",
is_flag=True,
Expand Down Expand Up @@ -1504,6 +1511,7 @@
prompt,
prompt_dsn,
list_databases,
ping_database,
auto_vertical_output,
list_dsn,
warn,
Expand Down Expand Up @@ -1581,8 +1589,8 @@
service = database[8:]
elif os.getenv("PGSERVICE") is not None:
service = os.getenv("PGSERVICE")
# because option --list or -l are not supposed to have a db name
if list_databases:
# because option --ping, --list or -l are not supposed to have a db name
if list_databases or ping_database:
database = "postgres"

if dsn != "":
Expand Down Expand Up @@ -1626,6 +1634,25 @@

sys.exit(0)

if ping_database:
results = None
try:
results = list(pgcli.pgexecute.run("SELECT 1"))
Fixed Show fixed Hide fixed
j-bennet marked this conversation as resolved.
Show resolved Hide resolved
except Exception:
Copy link
Member

Choose a reason for hiding this comment

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

I wonder what exception we could get, here. If pgcli cannot connect to the server (or use the database), we'll fail when calling PGCLI.connect() a few blocks before, not here.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, @dbaty, it's extremely unlikely, but not impossible. I've seen this once in my life, and not with postgresql; database server was up, but the transaction log was full. I'm ok with keeping the try/catch.

Copy link
Member

Choose a reason for hiding this comment

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

You're right, @dbaty, it's extremely unlikely, but not impossible. I've seen this once in my life, and not with postgresql; database server was up, but the transaction log was full.

@j-bennet : then it would be useful to log the exact error we got, instead of the misleading "could not connect to the database".

# yup, really, anything here
...
Fixed Show fixed Hide fixed
Fixed Show fixed Hide fixed

if results:
click.echo("PONG")
sys.exit(0)
else:
click.secho(
"Could not connect to the database. Please check that the database is running.",
err=True,
fg="red",
)
sys.exit(1)
j-bennet marked this conversation as resolved.
Show resolved Hide resolved

pgcli.logger.debug(
"Launch Params: \n" "\tdatabase: %r" "\tuser: %r" "\thost: %r" "\tport: %r",
database,
Expand Down
4 changes: 4 additions & 0 deletions tests/features/basic_commands.feature
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ Feature: run the cli,
When we list databases
then we see list of databases

Scenario: ping databases
When we ping the database
then we get a pong response

Scenario: run the cli with --username
When we launch dbcli using --username
and we send "\?" command
Expand Down
13 changes: 13 additions & 0 deletions tests/features/steps/basic_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,19 @@ def step_see_list_databases(context):
context.cmd_output = None


@when("we ping the database")
def step_ping_database(context):
cmd = ["pgcli", "--ping"]
context.cmd_output = subprocess.check_output(cmd, cwd=context.package_root)


@then("we get a pong response")
def step_get_pong_response(context):
# exit code 0 is implied by the presence of cmd_output here, which
# is only set on a successful run.
assert context.cmd_output.strip() == b"PONG", f"Output was {context.cmd_output}"


@when("we run dbcli")
def step_run_cli(context):
wrappers.run_cli(context)
Expand Down
Loading