Add version and data directory to initdb output

Started by David G. Johnstonover 3 years ago10 messages
#1David G. Johnston
david.g.johnston@gmail.com

Hackers,

initdb is already pretty chatty, and the version of the cluster being
installed seems useful to include as well. The data directory is probably
less so - though I am thinking that the absolute path would be useful to
report, especially when a relative path is specified (I didn't figure that
part out yet, figured I'd get the idea approved before working out how to
make it happen).

Moving "Success" to that "summary output" line and leaving the optional
shell command line just be the shell command made sense to me.

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index ab826da650..54a1d1fcac 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -3119,6 +3119,9 @@ main(int argc, char *argv[])
  "--auth-local and --auth-host, the next time you run initdb.");
  }
+ printf(_("\nSuccess. PostgreSQL version %s cluster has been initialized
at %s.\n"), PG_VERSION, pg_data);
+ fflush(stdout);
+
  if (!noinstructions)
  {
  /*
@@ -3147,7 +3150,7 @@ main(int argc, char *argv[])
  /* translator: This is a placeholder in a shell command. */
  appendPQExpBuffer(start_db_cmd, " -l %s start", _("logfile"));
- printf(_("\nSuccess. You can now start the database server using:\n\n"
+ printf(_("\nYou can now start the database server using:\n\n"
  "    %s\n\n"),
    start_db_cmd->data);

David J.

#2Daniel Gustafsson
daniel@yesql.se
In reply to: David G. Johnston (#1)
Re: Add version and data directory to initdb output

On 16 Apr 2022, at 01:50, David G. Johnston <david.g.johnston@gmail.com> wrote:

initdb is already pretty chatty, and the version of the cluster being installed seems useful to include as well.

That seems quite reasonable.

The data directory is probably less so - though I am thinking that the absolute path would be useful to report, especially when a relative path is specified (I didn't figure that part out yet, figured I'd get the idea approved before working out how to make it happen).

I'm less convinced that it will be worth the additional code to make it
portable across *nix/Windows etc.

Moving "Success" to that "summary output" line and leaving the optional shell command line just be the shell command made sense to me.

Looking at the output, couldn't it alternatively be printed grouped with the
other info on the cluster, ie the final three rows in the example below:

./bin/initdb -D data
The files belonging to this database system will be owned by user "<username>".
This user must also own the server process.

The database cluster will be initialized with locale "en_US.UTF-8".
The default database encoding has accordingly been set to "UTF8".
The default text search configuration will be set to "english".

How about 'The database cluster will be initialized with version "14.2".' added
there, which then can keep the "Success" line in place in case existing scripts
are triggering on that line?

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

#3David G. Johnston
david.g.johnston@gmail.com
In reply to: Daniel Gustafsson (#2)
Re: Add version and data directory to initdb output

On Tue, Apr 19, 2022 at 2:28 AM Daniel Gustafsson <daniel@yesql.se> wrote:

On 16 Apr 2022, at 01:50, David G. Johnston <david.g.johnston@gmail.com>

wrote:

initdb is already pretty chatty, and the version of the cluster being

installed seems useful to include as well.

That seems quite reasonable.

The data directory is probably less so - though I am thinking that the

absolute path would be useful to report, especially when a relative path is
specified (I didn't figure that part out yet, figured I'd get the idea
approved before working out how to make it happen).

I'm less convinced that it will be worth the additional code to make it
portable across *nix/Windows etc.

ok

Moving "Success" to that "summary output" line and leaving the optional

shell command line just be the shell command made sense to me.

Looking at the output, couldn't it alternatively be printed grouped with
the
other info on the cluster, ie the final three rows in the example below:

./bin/initdb -D data
The files belonging to this database system will be owned by user
"<username>".
This user must also own the server process.

The database cluster will be initialized with locale "en_US.UTF-8".
The default database encoding has accordingly been set to "UTF8".
The default text search configuration will be set to "english".

How about 'The database cluster will be initialized with version "14.2".'
added
there, which then can keep the "Success" line in place in case existing
scripts
are triggering on that line?

The motivating situation had me placing it as close to the last line as
possible so my 8 line or so tmux panel would show it to me without
scrolling. The version is all I cared about, but when writing the patch
the path seemed to be at least worth considering.

As for "Success", I'm confused about the --no-instructions choice to change
it the way it did, but given that precedent I only felt it important to
leave the word Success as the leading word on a line. Scripts should be
triggering on the exit code anyway and presently --no-instructions removes
the Success acknowledgement completely anyway.

David J.

#4Daniel Gustafsson
daniel@yesql.se
In reply to: David G. Johnston (#3)
Re: Add version and data directory to initdb output

On 19 Apr 2022, at 15:56, David G. Johnston <david.g.johnston@gmail.com> wrote:

The motivating situation had me placing it as close to the last line as possible so my 8 line or so tmux panel would show it to me without scrolling. The version is all I cared about, but when writing the patch the path seemed to be at least worth considering.

As for "Success", I'm confused about the --no-instructions choice to change it the way it did, but given that precedent I only felt it important to leave the word Success as the leading word on a line. Scripts should be triggering on the exit code anyway and presently --no-instructions removes the Success acknowledgement completely anyway.

Good point, I forgot about the no-instructions option.

./daniel

#5Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: David G. Johnston (#3)
Re: Add version and data directory to initdb output

On 19.04.22 15:55, David G. Johnston wrote:

The motivating situation had me placing it as close to the last line as
possible so my 8 line or so tmux panel would show it to me without
scrolling.  The version is all I cared about, but when writing the patch
the path seemed to be at least worth considering.

As for "Success", I'm confused about the --no-instructions choice to
change it the way it did, but given that precedent I only felt it
important to leave the word Success as the leading word on a line.
Scripts should be triggering on the exit code anyway and presently
--no-instructions removes the Success acknowledgement completely anyway.

The order of outputs of initdb seems to be approximately

1. These are the settings I will use based on what you told me.
2. This is what I'm doing right now.
3. Here's what you can do next.

Your additions would appear to fall into bucket #1. So I think adding
them near the start of the output makes more sense. Otherwise, one
could also argue that all the locale information etc. should also be
repeated at the end, in case one forgot them or whatever.

#6David G. Johnston
david.g.johnston@gmail.com
In reply to: Peter Eisentraut (#5)
Re: Add version and data directory to initdb output

On Wed, Apr 20, 2022 at 2:04 PM Peter Eisentraut <
peter.eisentraut@enterprisedb.com> wrote:

On 19.04.22 15:55, David G. Johnston wrote:

The motivating situation had me placing it as close to the last line as
possible so my 8 line or so tmux panel would show it to me without
scrolling. The version is all I cared about, but when writing the patch
the path seemed to be at least worth considering.

As for "Success", I'm confused about the --no-instructions choice to
change it the way it did, but given that precedent I only felt it
important to leave the word Success as the leading word on a line.
Scripts should be triggering on the exit code anyway and presently
--no-instructions removes the Success acknowledgement completely anyway.

The order of outputs of initdb seems to be approximately

1. These are the settings I will use based on what you told me.
2. This is what I'm doing right now.
3. Here's what you can do next.

Your additions would appear to fall into bucket #1. So I think adding
them near the start of the output makes more sense. Otherwise, one
could also argue that all the locale information etc. should also be
repeated at the end, in case one forgot them or whatever.

I agree with the observation but it initdb is fast enough and
non-interactive and so that order isn't particularly appealing.

Thus either:

1. Initialization is running ... Here's what we are doing.
2. All done! Here's what we did.
3. Here's what you can do next.

or

1. These are the settings I will use based on what you told me.
2. This is what I'm doing right now.
3. All done! Here's what you ended up with (can repeat items from 1 if
desired...)
4. Here's what you can do next.

I'd rather do the first proposal given buy-in. Though I would have
concerns about what the output looks like upon failure.

I'm basically proposing the second option, add a formal "All done!" section
and recap what the final result is. I'd be content with having the version
appear in both 1 and 3 in that scenario. It isn't a frequently executed
command, already is verbose, and when done interactively in development I
don't want to have to dedicate a 20 line panel so I can see "All Done!" and
some (one) key attribute(s) (locale and path seems useful though) without
scrolling.

If the consensus is to place it before, and only before, the "this is what
I'm doing right now" stuff, that is better than nothing, but the choice of
not doing so was intentional.

David J.

#7Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: David G. Johnston (#6)
Re: Add version and data directory to initdb output

On 20.04.22 23:21, David G. Johnston wrote:

I agree with the observation but it initdb is fast enough and
non-interactive and so that order isn't particularly appealing.

I'm not a particular fan of the current initdb output and it could use a
general revision IMO. If you want to look into that, please do. But
for your particular proposed addition, let's put it somewhere it makes
sense either in the current scheme or a future scheme when that is done.

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#7)
Re: Add version and data directory to initdb output

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

I'm not a particular fan of the current initdb output and it could use a
general revision IMO. If you want to look into that, please do. But
for your particular proposed addition, let's put it somewhere it makes
sense either in the current scheme or a future scheme when that is done.

TBH, I think we should reject the current proposal outright.
The target directory's name already appears twice in initdb's output;
we do not need a third time. And as for the version, if you want that
you can get it from "initdb --version".

I agree that there could be scope for rethinking initdb's output
altogether. It's fast enough nowadays that the former need for
progress reporting could probably be dropped. Maybe we could
go over to something that's more nearly intended to be a
machine-readable summary of the configuration, like

Data directory: ...
Owning user ID: ...
Locale: ...
Default server encoding: ...
etc etc

Even if you like the current output for interactive usage,
perhaps something like this could be selected by a switch for
non-interactive usage (or just repurpose --no-instructions).

regards, tom lane

#9Michael Banck
michael.banck@credativ.de
In reply to: Tom Lane (#8)
Re: Add version and data directory to initdb output

Hi,

On Thu, Apr 21, 2022 at 10:18:56AM -0400, Tom Lane wrote:

And as for the version, if you want that you can get it from "initdb
--version".

I assumed the point in stamping the version in the output was that
people might want to pipe it to some logfile and then later on, when
they found some issues, be able to go back and know what version was
used when initializing this data directory.

Michael

#10David G. Johnston
david.g.johnston@gmail.com
In reply to: Tom Lane (#8)
Re: Add version and data directory to initdb output

On Thu, Apr 21, 2022 at 7:18 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:

I'm not a particular fan of the current initdb output and it could use a
general revision IMO. If you want to look into that, please do. But
for your particular proposed addition, let's put it somewhere it makes
sense either in the current scheme or a future scheme when that is done.

TBH, I think we should reject the current proposal outright.
The target directory's name already appears twice in initdb's output;
we do not need a third time. And as for the version, if you want that
you can get it from "initdb --version".

I don't really see a reason not to add the version to the log output, if
just for simplicity and having a self-contained stream of content.

I'm off my desire to have it be the nearly last thing to print though;
having it print first actually works better since if you are interactive
you'll see it pop-up just after pressing enter. Subconsciously you'll know
what you are expecting to see there and if it just happens to be different
you'll probably notice it. Solutions requiring additional commands/effort
to retrieve the version presume one is expecting/caring about checking that
value specifically, and while that may be true the simplicity combined with
the benefit to people not expecting there to be an issue make adding it
alongside the various others key=value settings a no-brainer for me.

David J.