Thread: Add version and data directory to initdb output
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);
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.
> 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, especiallywhen a relative path is specified (I didn't figure that part out yet, figured I'd get the idea approved beforeworking 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 madesense 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/
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.
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
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.
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.
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.
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.
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.
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
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
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.