Skip to content

Commit 1f1b601

Browse files
committed
Refactor pgBackRest error handling for clarity
- Replace magic number 82 with PGBACKREST_ARCHIVE_TIMEOUT_ERROR_CODE constant - Use semicolon separator for multiple error messages (improved readability) - Document TimeoutError handling to preserve error state for juju resolve - Update corresponding unit tests Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
1 parent b6819f1 commit 1f1b601

File tree

3 files changed

+9
-3
lines changed

3 files changed

+9
-3
lines changed

src/backups.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
BACKUP_TYPE_OVERRIDES,
3737
BACKUP_USER,
3838
PATRONI_CONF_PATH,
39+
PGBACKREST_ARCHIVE_TIMEOUT_ERROR_CODE,
3940
PGBACKREST_BACKUP_ID_FORMAT,
4041
PGBACKREST_CONF_PATH,
4142
PGBACKREST_CONFIGURATION_FILE,
@@ -395,7 +396,7 @@ def _extract_error_message(stdout: str, stderr: str) -> str:
395396

396397
# If we found error/warning lines, return them joined
397398
if error_lines:
398-
return " ".join(error_lines)
399+
return "; ".join(error_lines)
399400

400401
# Otherwise return the last non-empty line from stderr or stdout
401402
if stderr.strip():
@@ -737,7 +738,7 @@ def check_stanza(self) -> bool:
737738
f"--stanza={self.stanza_name}",
738739
"check",
739740
])
740-
if return_code == 82:
741+
if return_code == PGBACKREST_ARCHIVE_TIMEOUT_ERROR_CODE:
741742
# Raise an error if the archive command timeouts, so the user has the possibility
742743
# to fix network issues and call juju resolve to re-trigger the hook that calls
743744
# this method.
@@ -750,6 +751,7 @@ def check_stanza(self) -> bool:
750751
raise Exception(stderr)
751752
self.charm._set_primary_status_message()
752753
except TimeoutError as e:
754+
# Re-raise to put charm in error state (not blocked), allowing juju resolve
753755
raise e
754756
except Exception as e:
755757
# If the check command doesn't succeed, remove the stanza name

src/constants.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@
3232

3333
# Snap constants.
3434
PGBACKREST_EXECUTABLE = "charmed-postgresql.pgbackrest"
35+
# pgBackRest error codes
36+
PGBACKREST_ARCHIVE_TIMEOUT_ERROR_CODE = (
37+
82 # Archive timeout - unable to archive WAL files within configured timeout period
38+
)
3539
POSTGRESQL_SNAP_NAME = "charmed-postgresql"
3640
SNAP_PACKAGES = [
3741
(

tests/unit/test_backups.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def test_extract_error_message_with_multiple_errors():
7878
P00 ERROR: second error"""
7979
stderr = ""
8080
result = PostgreSQLBackups._extract_error_message(stdout, stderr)
81-
assert result == "ERROR: first error WARN: warning message ERROR: second error"
81+
assert result == "ERROR: first error; WARN: warning message; ERROR: second error"
8282

8383

8484
def test_extract_error_message_with_empty_output():

0 commit comments

Comments
 (0)