Skip to content

fix(backups): configure pgBackRest stderr logging#1750

Open
nanookclaw wants to merge 1 commit into
canonical:mainfrom
nanookclaw:fix/pgbackrest-log-level-config
Open

fix(backups): configure pgBackRest stderr logging#1750
nanookclaw wants to merge 1 commit into
canonical:mainfrom
nanookclaw:fix/pgbackrest-log-level-config

Conversation

@nanookclaw

Copy link
Copy Markdown

Summary

Adds log-level-stderr=warn to the rendered pgBackRest global configuration so the stderr logging level is configured once for every pgBackRest invocation that uses pgbackrest.conf, as requested in #1353.

The existing --log-level-console=debug backup argument is left unchanged because it controls console output for the backup action and is distinct from the stderr logging level being moved into the config file.

Verification

  • python3 -m compileall -q tests/unit/test_backups.py
  • ruff check tests/unit/test_backups.py
  • ruff format --check tests/unit/test_backups.py
  • git diff --check
  • Jinja render smoke check confirmed log-level-stderr=warn appears in the rendered config

Could not run the focused pytest locally because this container lacks the project test environment: host pytest autoloads a missing evalcraft plugin, plugin-autoload-disabled pytest then lacks botocore, and poetry/tox are not installed here.

Closes #1353

@nanookclaw nanookclaw requested a review from a team as a code owner June 6, 2026 00:39
@nanookclaw nanookclaw requested review from carlcsaposs-canonical, dragomirp, juju-charm-bot, marceloneppel and taurus-forever and removed request for a team June 6, 2026 00:39

@taurus-forever taurus-forever left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this

PGBACKREST_LOG_LEVEL_STDERR = "--log-level-stderr=warn"
be reverted (as discussed here) ?

@nanookclaw

Copy link
Copy Markdown
Author

I don’t think a separate constant needs to be reverted in this PR. The command-level PGBACKREST_LOG_LEVEL_STDERR approach from #1320 is not present on current main; src/constants.py#L34 is just PGBACKREST_EXECUTABLE now.

This PR applies the earlier suggestion in the config layer instead: templates/pgbackrest.conf.j2 now sets log-level-stderr=warn, so pgBackRest commands using --config=.../pgbackrest.conf pick it up without adding a per-command flag.

The only remaining command-level logging flag I see on main is _run_backup()’s --log-level-console=debug, which is backup-specific and looks intentionally preserved for backup log capture. If you want this PR to also remove or change that backup-specific flag, I can adjust it, but I kept the diff scoped to the shared pgBackRest config.

@carlcsaposs-canonical

Copy link
Copy Markdown
Contributor

@taurus-forever perhaps we should set the default branch to 16/edge?

@taurus-forever

Copy link
Copy Markdown
Contributor

@taurus-forever perhaps we should set the default branch to 16/edge?

It was pending stereo mode PRs release to perform massive default branch upgrade as discussed in Madrid.
It is coming ...

@marceloneppel marceloneppel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I created #1767 to also work on 16/edge and remove the constant @taurus-forever mentioned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move --log-level-stderr=warn to pgBackRest's configuration file

4 participants