Closed mikewallace1979 closed 9 months ago
This PR definitely looks better than the previous one. I'm posting a few thoughts.
Thanks for reviewing the draft PR @barthisrael - I implemented all of your suggestions so far 😄
There will be a few follow additional commits with unit tests and docs, plus any other changes we determine we need as we continue to think about this patch.
Kudos, no new issues were introduced!
0 New issues
0 Security Hotspots
98.6% Coverage on New Code
0.0% Duplication on New Code
This is functionally-equivalent to #867 however the different conninfo strings for WAL streaming are managed before creating the server.
The general idea is that:
barman backup
and most other barman commands createbarman.server.Server
objects using theconninfo
andstreaming_conninfo
in the configuration as usual.barman receive-wal
andbarman cron
createbarman.server.Server
objects usingwal_conninfo
andwal_streaming_conninfo
if they are set but useconninfo
andstreaming_conninfo
otherwise.barman replication-status
will use either, depending on the value of its--source
option (which defaults to the backup conninfo, to preserve existing behaviour).This works quite nicely however:
barman cron
need to use the backup-host conninfo strings for some reason? Are there other barman commands which should have the WAL conninfo?barman check
still needs work because it is checking the complete configuration of the Barman server.Point 2 is addressed by the second commit in this PR where we make
barman check
get the WAL streaming status and carry out the relevant checks using that.For point 1 we can do some careful review.
Overall this seems cleaner than #867 and does not require the scattergun changes to
barman.server.Server
which are present in that PR.Related: Integration tests.