trivial improvement to system_or_bail

Started by Alvaro Herreraalmost 5 years ago3 messageshackers
Jump to latest
#1Alvaro Herrera
alvherre@2ndquadrant.com

When PostgresNode::system_or_bail() fails, it's quite opaque as to what
is happening. This patch improves things by printing some detail, as
suggested in Perl's doc for system().

--
�lvaro Herrera 39�49'30"S 73�17'W
https://www.EnterpriseDB.com/

Attachments:

system_or_bail.patchtext/x-diff; charset=us-asciiDownload+12-1
#2Daniel Gustafsson
daniel@yesql.se
In reply to: Alvaro Herrera (#1)
Re: trivial improvement to system_or_bail

On 30 Jun 2021, at 17:24, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

When PostgresNode::system_or_bail() fails, it's quite opaque as to what
is happening. This patch improves things by printing some detail, as
suggested in Perl's doc for system().

+1 on this from reading the patch.

+ BAIL_OUT("system $_[0] failed: $!\n");
I wonder if we should take more inspiration from the Perl manual and change it
to "failed to execute" to make it clear that the failure was in executing the
program, not from the program itself?

--
Daniel Gustafsson https://vmware.com/

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#1)
Re: trivial improvement to system_or_bail

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

When PostgresNode::system_or_bail() fails, it's quite opaque as to what
is happening. This patch improves things by printing some detail, as
suggested in Perl's doc for system().

+1 for adding the extra details, but another thing that I've always found
very confusing is just the phrasing of the message itself. It makes
no sense unless (a) you know that "system" is Perl's function for
executing a shell command, (b) you are familiar with Perl's generally
cavalier approach to parentheses, and (c) you are also unbothered by
whether the word "failed" is part of the message text or the command
being complained of. We really need to do something to set off the
shell command's text from the surrounding verbiage a little better.

I'd prefer something like

command "pg_ctl start" failed: details here

regards, tom lane