public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dagfinn Ilmari Mannsåker <[email protected]>
To: Peter Smith <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: pg_createsubscriber TAP test wrapping makes command options hard to read.
Date: Thu, 12 Dec 2024 13:17:48 +0000
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAHut+PvfaUZQR3rEnj-Dcg_6713Wu4yRdd-yZkkC9NVsARht8A@mail.gmail.com>
<[email protected]>
<CAHut+Ps6Q+i_3xS6H7ybw3-hx9+2QOuneTJwHArxqxFnKX5+4g@mail.gmail.com>
<[email protected]>
<CAHut+PvFGytgJT+K_e2x+uWKuVYCBR3HNaNtPKYEkO3YxAwZvw@mail.gmail.com>
<[email protected]>
<CAHut+PvYsVhF5k-mYHVEFdpijdgkYRtoeWVxdspxio0ePRpkug@mail.gmail.com>
<[email protected]>
Dagfinn Ilmari Mannsåker <[email protected]> writes:
> Peter Smith <[email protected]> writes:
>
>> On Thu, Dec 12, 2024 at 2:53 PM Michael Paquier <[email protected]> wrote:
>> ...
>>
>>> > So, AFAICT I can workaround the perltidy wrapping just by putting all
>>> > the noarg options at the bottom of the command, then all the
>>> > option/optarg pairs (ie 2s) will stay together. I can post another
>>> > patch to do it this way unless you think it is too hacky.
>>>
>>> This trick works for me if that makes the long list of option easier
>>> to read. With two elements of the array perl line, I would just put
>>> some --dry-run or --verbose at the end of their respective arrays.
>>> --
>>> Michael
>>
>> Hi Michael.
>>
>> Yes, that is the workaround that I was proposing.
>
> A better option, IMO, is to use the fat comma (=>) between options and
> their values. This makes it clear both to humans and perltidy that they
> belong together, and we can put all the valueless options first without
> things being rewrapped.
Here's a more thorough patch, that also applies the fat comma treatment
to other pg_createsubscriber invocations in the same file that don't
currently happen to be mangled by perltidy. It also adds trailing
commas to the last item in multi-line command arrays, which is common
perl style.
- ilmari
Attachments:
[text/x-diff] v4-0001-Modify-wrapping-to-make-command-options-easier-to.patch (10.5K, ../[email protected]/2-v4-0001-Modify-wrapping-to-make-command-options-easier-to.patch)
download | inline diff:
From 953d0c8ca8202d6f53af833fd53e3f6b1929fa77 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <[email protected]>
Date: Thu, 12 Dec 2024 11:56:15 +0000
Subject: [PATCH v4] Modify wrapping to make command options easier to read
Use fat comma after options that take values, both to make it clearer
to humans, and to stop perltidy from re-wrapping them.
Also remove pointless quoting of $PostgreSQL::Test::Utils::timeout_default.
---
.../t/040_pg_createsubscriber.pl | 255 +++++++++---------
1 file changed, 135 insertions(+), 120 deletions(-)
diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
index 0a900edb656..369846db0d0 100644
--- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
+++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
@@ -46,69 +46,75 @@ sub generate_db
command_fails(['pg_createsubscriber'],
'no subscriber data directory specified');
command_fails(
- [ 'pg_createsubscriber', '--pgdata', $datadir ],
+ [ 'pg_createsubscriber', '--pgdata' => $datadir ],
'no publisher connection string specified');
command_fails(
[
- 'pg_createsubscriber', '--verbose',
- '--pgdata', $datadir,
- '--publisher-server', 'port=5432'
+ 'pg_createsubscriber',
+ '--verbose',
+ '--pgdata' => $datadir,
+ '--publisher-server' => 'port=5432',
],
'no database name specified');
command_fails(
[
- 'pg_createsubscriber', '--verbose',
- '--pgdata', $datadir,
- '--publisher-server', 'port=5432',
- '--database', 'pg1',
- '--database', 'pg1'
+ 'pg_createsubscriber',
+ '--verbose',
+ '--pgdata' => $datadir,
+ '--publisher-server' => 'port=5432',
+ '--database' => 'pg1',
+ '--database' => 'pg1',
],
'duplicate database name');
command_fails(
[
- 'pg_createsubscriber', '--verbose',
- '--pgdata', $datadir,
- '--publisher-server', 'port=5432',
- '--publication', 'foo1',
- '--publication', 'foo1',
- '--database', 'pg1',
- '--database', 'pg2'
+ 'pg_createsubscriber',
+ '--verbose',
+ '--pgdata' => $datadir,
+ '--publisher-server' => 'port=5432',
+ '--publication' => 'foo1',
+ '--publication' => 'foo1',
+ '--database' => 'pg1',
+ '--database' => 'pg2',
],
'duplicate publication name');
command_fails(
[
- 'pg_createsubscriber', '--verbose',
- '--pgdata', $datadir,
- '--publisher-server', 'port=5432',
- '--publication', 'foo1',
- '--database', 'pg1',
- '--database', 'pg2'
+ 'pg_createsubscriber',
+ '--verbose',
+ '--pgdata' => $datadir,
+ '--publisher-server' => 'port=5432',
+ '--publication' => 'foo1',
+ '--database' => 'pg1',
+ '--database' => 'pg2',
],
'wrong number of publication names');
command_fails(
[
- 'pg_createsubscriber', '--verbose',
- '--pgdata', $datadir,
- '--publisher-server', 'port=5432',
- '--publication', 'foo1',
- '--publication', 'foo2',
- '--subscription', 'bar1',
- '--database', 'pg1',
- '--database', 'pg2'
+ 'pg_createsubscriber',
+ '--verbose',
+ '--pgdata' => $datadir,
+ '--publisher-server' => 'port=5432',
+ '--publication' => 'foo1',
+ '--publication' => 'foo2',
+ '--subscription' => 'bar1',
+ '--database' => 'pg1',
+ '--database' => 'pg2',
],
'wrong number of subscription names');
command_fails(
[
- 'pg_createsubscriber', '--verbose',
- '--pgdata', $datadir,
- '--publisher-server', 'port=5432',
- '--publication', 'foo1',
- '--publication', 'foo2',
- '--subscription', 'bar1',
- '--subscription', 'bar2',
- '--replication-slot', 'baz1',
- '--database', 'pg1',
- '--database', 'pg2'
+ 'pg_createsubscriber',
+ '--verbose',
+ '--pgdata' => $datadir,
+ '--publisher-server' => 'port=5432',
+ '--publication' => 'foo1',
+ '--publication' => 'foo2',
+ '--subscription' => 'bar1',
+ '--subscription' => 'bar2',
+ '--replication-slot' => 'baz1',
+ '--database' => 'pg1',
+ '--database' => 'pg2',
],
'wrong number of replication slot names');
@@ -168,41 +174,44 @@ sub generate_db
# Run pg_createsubscriber on a promoted server
command_fails(
[
- 'pg_createsubscriber', '--verbose',
- '--dry-run', '--pgdata',
- $node_t->data_dir, '--publisher-server',
- $node_p->connstr($db1), '--socketdir',
- $node_t->host, '--subscriber-port',
- $node_t->port, '--database',
- $db1, '--database',
- $db2
+ 'pg_createsubscriber',
+ '--verbose',
+ '--dry-run',
+ '--pgdata' => $node_t->data_dir,
+ '--publisher-server' => $node_p->connstr($db1),
+ '--socketdir' => $node_t->host,
+ '--subscriber-port' => $node_t->port,
+ '--database' => $db1,
+ '--database' => $db2,
],
'target server is not in recovery');
# Run pg_createsubscriber when standby is running
command_fails(
[
- 'pg_createsubscriber', '--verbose',
- '--dry-run', '--pgdata',
- $node_s->data_dir, '--publisher-server',
- $node_p->connstr($db1), '--socketdir',
- $node_s->host, '--subscriber-port',
- $node_s->port, '--database',
- $db1, '--database',
- $db2
+ 'pg_createsubscriber',
+ '--verbose',
+ '--dry-run',
+ '--pgdata' => $node_s->data_dir,
+ '--publisher-server' => $node_p->connstr($db1),
+ '--socketdir' => $node_s->host,
+ '--subscriber-port' => $node_s->port,
+ '--database' => $db1,
+ '--database' => $db2,
],
'standby is up and running');
# Run pg_createsubscriber on about-to-fail node F
command_fails(
[
- 'pg_createsubscriber', '--verbose',
- '--pgdata', $node_f->data_dir,
- '--publisher-server', $node_p->connstr($db1),
- '--socketdir', $node_f->host,
- '--subscriber-port', $node_f->port,
- '--database', $db1,
- '--database', $db2
+ 'pg_createsubscriber',
+ '--verbose',
+ '--pgdata' => $node_f->data_dir,
+ '--publisher-server' => $node_p->connstr($db1),
+ '--socketdir' => $node_f->host,
+ '--subscriber-port' => $node_f->port,
+ '--database' => $db1,
+ '--database' => $db2
],
'subscriber data directory is not a copy of the source database cluster');
@@ -216,14 +225,15 @@ sub generate_db
# Run pg_createsubscriber on node C (P -> S -> C)
command_fails(
[
- 'pg_createsubscriber', '--verbose',
- '--dry-run', '--pgdata',
- $node_c->data_dir, '--publisher-server',
- $node_s->connstr($db1), '--socketdir',
- $node_c->host, '--subscriber-port',
- $node_c->port, '--database',
- $db1, '--database',
- $db2
+ 'pg_createsubscriber',
+ '--verbose',
+ '--dry-run',
+ '--pgdata' => $node_c->data_dir,
+ '--publisher-server' => $node_s->connstr($db1),
+ '--socketdir' => $node_c->host,
+ '--subscriber-port' => $node_c->port,
+ '--database' => $db1,
+ '--database' => $db2,
],
'primary server is in recovery');
@@ -239,14 +249,16 @@ sub generate_db
$node_s->stop;
command_fails(
[
- 'pg_createsubscriber', '--verbose',
- '--dry-run', '--pgdata',
- $node_s->data_dir, '--publisher-server',
- $node_p->connstr($db1), '--socketdir',
- $node_s->host, '--subscriber-port',
- $node_s->port, '--database',
- $db1, '--database',
- $db2
+ 'pg_createsubscriber',
+ '--verbose',
+ '--dry-run',
+ '--pgdata' => $node_s->data_dir,
+ '--publisher-server' => $node_p->connstr($db1),
+ '--socketdir' => $node_s->host,
+ '--subscriber-port' => $node_s->port,
+ '--database' => $db1,
+ '--database' => $db2,
+
],
'primary contains unmet conditions on node P');
# Restore default settings here but only apply it after testing standby. Some
@@ -268,14 +280,15 @@ sub generate_db
});
command_fails(
[
- 'pg_createsubscriber', '--verbose',
- '--dry-run', '--pgdata',
- $node_s->data_dir, '--publisher-server',
- $node_p->connstr($db1), '--socketdir',
- $node_s->host, '--subscriber-port',
- $node_s->port, '--database',
- $db1, '--database',
- $db2
+ 'pg_createsubscriber',
+ '--verbose',
+ '--dry-run',
+ '--pgdata' => $node_s->data_dir,
+ '--publisher-server' => $node_p->connstr($db1),
+ '--socketdir' => $node_s->host,
+ '--subscriber-port' => $node_s->port,
+ '--database' => $db1,
+ '--database' => $db2,
],
'standby contains unmet conditions on node S');
$node_s->append_conf(
@@ -321,19 +334,20 @@ sub generate_db
# dry run mode on node S
command_ok(
[
- 'pg_createsubscriber', '--verbose',
- '--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default",
- '--dry-run', '--pgdata',
- $node_s->data_dir, '--publisher-server',
- $node_p->connstr($db1), '--socketdir',
- $node_s->host, '--subscriber-port',
- $node_s->port, '--publication',
- 'pub1', '--publication',
- 'pub2', '--subscription',
- 'sub1', '--subscription',
- 'sub2', '--database',
- $db1, '--database',
- $db2
+ 'pg_createsubscriber',
+ '--verbose',
+ '--dry-run',
+ '--recovery-timeout' => $PostgreSQL::Test::Utils::timeout_default,
+ '--pgdata' => $node_s->data_dir,
+ '--publisher-server' => $node_p->connstr($db1),
+ '--socketdir' => $node_s->host,
+ '--subscriber-port' => $node_s->port,
+ '--publication' => 'pub1',
+ '--publication' => 'pub2',
+ '--subscription' => 'sub1',
+ '--subscription' => 'sub2',
+ '--database' => $db1,
+ '--database' => $db2,
],
'run pg_createsubscriber --dry-run on node S');
@@ -346,32 +360,33 @@ sub generate_db
# pg_createsubscriber can run without --databases option
command_ok(
[
- 'pg_createsubscriber', '--verbose',
- '--dry-run', '--pgdata',
- $node_s->data_dir, '--publisher-server',
- $node_p->connstr($db1), '--socketdir',
- $node_s->host, '--subscriber-port',
- $node_s->port, '--replication-slot',
- 'replslot1'
+ 'pg_createsubscriber',
+ '--verbose',
+ '--dry-run',
+ '--pgdata' => $node_s->data_dir,
+ '--publisher-server' => $node_p->connstr($db1),
+ '--socketdir' => $node_s->host,
+ '--subscriber-port' => $node_s->port,
+ '--replication-slot' => 'replslot1',
],
'run pg_createsubscriber without --databases');
# Run pg_createsubscriber on node S
command_ok(
[
- 'pg_createsubscriber', '--verbose',
- '--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default",
- '--verbose', '--pgdata',
- $node_s->data_dir, '--publisher-server',
- $node_p->connstr($db1), '--socketdir',
- $node_s->host, '--subscriber-port',
- $node_s->port, '--publication',
- 'pub1', '--publication',
- 'Pub2', '--replication-slot',
- 'replslot1', '--replication-slot',
- 'replslot2', '--database',
- $db1, '--database',
- $db2
+ 'pg_createsubscriber',
+ '--verbose',
+ '--recovery-timeout' => $PostgreSQL::Test::Utils::timeout_default,
+ '--pgdata' => $node_s->data_dir,
+ '--publisher-server' => $node_p->connstr($db1),
+ '--socketdir' => $node_s->host,
+ '--subscriber-port' => $node_s->port,
+ '--publication' => 'pub1',
+ '--publication' => 'pub2',
+ '--replication-slot' => 'replslot1',
+ '--replication-slot' => 'replslot2',
+ '--database' => $db1,
+ '--database' => $db2,
],
'run pg_createsubscriber on node S');
--
2.47.1
view thread (20+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: pg_createsubscriber TAP test wrapping makes command options hard to read.
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox