diff --git a/barman/infofile.py b/barman/infofile.py index c88897615..15267390d 100644 --- a/barman/infofile.py +++ b/barman/infofile.py @@ -593,6 +593,94 @@ def set_attribute(self, key, value): """ setattr(self, key, value) + @property + def is_incremental(self): + """ + Only checks if the backup_info is an incremental backup + + .. note:: + This property only makes sense in the context of local backups stored in the + Barman server. However, this property is used for retention policies + processing, code which is shared among local and cloud backups. As this + property always returns ``False`` for cloud backups, it can safely be reused + in their code paths as well, though. + + :return bool: ``True`` if this backup has a parent, ``False`` otherwise. + """ + return self.parent_backup_id is not None + + @property + def has_children(self): + """ + Only checks if the backup_info has children + + .. note:: + This property only makes sense in the context of local backups stored in the + Barman server. However, this property is used for retention policies + processing, code which is shared among local and cloud backups. As this + property always returns ``False`` for cloud backups, it can safely be reused + in their code paths as well, though. + + :return bool: ``True`` if this backup has at least one child, ``False`` otherwise. + """ + return self.children_backup_ids is not None + + @property + def backup_type(self): + """ + Returns a string with the backup type label. + + .. note:: + Even though this property is available in this base class, it is not + expected to be used in the context of cloud backups. + + The backup type can be one of the following: + - ``snapshot``: If the backup mode starts with ``snapshot``. + - ``rsync``: If the backup mode starts with ``rsync``. + - ``incremental``: If the mode is ``postgres`` and the backup is incremental. + - ``full``: If the mode is ``postgres`` and the backup is not incremental. + + :return str: The backup type label. + """ + if self.mode.startswith("snapshot"): + return "snapshot" + elif self.mode.startswith("rsync"): + return "rsync" + return "incremental" if self.is_incremental else "full" + + @property + def deduplication_ratio(self): + """ + Returns a value between and including ``0`` and ``1`` related to the estimate + deduplication ratio of the backup. + + .. note:: + For ``rsync`` backups, the :attr:`size` of the backup, which is the sum of + all file sizes in basebackup directory, is used to calculate the + ratio. For ``postgres`` backups, the :attr:`cluster_size` is used, which contains + the estimated size of the Postgres cluster at backup time. + + We perform this calculation to make an estimation of how much network and disk + I/O has been saved when taking an incremental backup through ``rsync`` or through + ``pg_basebackup``. + + We abuse of the term "deduplication" here. It makes more sense to ``rsync`` than to + ``postgres`` method. However, the idea is the same in both cases: get an estimation + of resources saving. + + .. note:: + Even though this property is available in this base class, it is not + expected to be used in the context of cloud backups. + + :return float: The backup deduplication ratio. + """ + size = self.cluster_size + if self.backup_type == "rsync": + size = self.size + if size and self.deduplicated_size: + return 1 - (self.deduplicated_size / size) + return 0 + def to_dict(self): """ Return the backup_info content as a simple dictionary @@ -733,72 +821,6 @@ def __init__(self, server, info_file=None, backup_id=None, **kwargs): e, ) - @property - def is_incremental(self): - """ - Only checks if the backup_info is an incremental backup - - :return bool: ``True`` if this backup has a parent, ``False`` otherwise. - """ - return self.parent_backup_id is not None - - @property - def has_children(self): - """ - Only checks if the backup_info has children - - :return bool: ``True`` if this backup has at least one child, ``False`` otherwise. - """ - return self.children_backup_ids is not None - - @property - def backup_type(self): - """ - Returns a string with the backup type label. - - The backup type can be one of the following: - - ``snapshot``: If the backup mode starts with ``snapshot``. - - ``rsync``: If the backup mode starts with ``rsync``. - - ``incremental``: If the mode is ``postgres`` and the backup is incremental. - - ``full``: If the mode is ``postgres`` and the backup is not incremental. - - :return str: The backup type label. - """ - if self.mode.startswith("snapshot"): - return "snapshot" - elif self.mode.startswith("rsync"): - return "rsync" - return "incremental" if self.is_incremental else "full" - - @property - def deduplication_ratio(self): - """ - Returns a value between and including ``0`` and ``1`` related to the estimate - deduplication ratio of the backup. - - .. note:: - For ``rsync`` backups, the :attr:`size` of the backup, which is the sum of - all file sizes in basebackup directory, is used to calculate the - ratio. For ``postgres`` backups, the :attr:`cluster_size` is used, which contains - the estimated size of the Postgres cluster at backup time. - - We perform this calculation to make an estimation of how much network and disk - I/O has been saved when taking an incremental backup through ``rsync`` or through - ``pg_basebackup``. - - We abuse of the term "deduplication" here. It makes more sense to ``rsync`` than to - ``postgres`` method. However, the idea is the same in both cases: get an estimation - of resources saving. - - :return float: The backup deduplication ratio. - """ - size = self.cluster_size - if self.backup_type == "rsync": - size = self.size - if size and self.deduplicated_size: - return 1 - (self.deduplicated_size / size) - return 0 - def get_list_of_files(self, target): """ Get the list of files for the current backup diff --git a/tests/test_infofile.py b/tests/test_infofile.py index 7b5954afa..d19ad589c 100644 --- a/tests/test_infofile.py +++ b/tests/test_infofile.py @@ -520,6 +520,24 @@ def test_backup_info_from_backup_id(self, tmpdir): assert b_info.tablespaces[0].oid == 16384 assert b_info.tablespaces[0].location == "/fake_tmp/tbs" + @pytest.mark.parametrize( + ("mode", "parent_backup_id", "expected_backup_type"), + [ + ("rsync", None, "rsync"), + ("postgres", "some_id", "incremental"), + ("postgres", None, "full"), + ("snapshot", None, "snapshot"), + ], + ) + def test_backup_type(self, mode, parent_backup_id, expected_backup_type): + """ + Ensure :meth:`LocalBackupInfo.backup_type` returns the correct backup type label. + """ + backup_info = build_test_backup_info(parent_backup_id=parent_backup_id) + backup_info.mode = mode + + assert backup_info.backup_type == expected_backup_type + def test_backup_info_save(self, tmpdir): """ Test the save method of a BackupInfo object @@ -947,7 +965,7 @@ def test_get_backup_manifest_path(self, mock_get_data_dir, backup_info): mock_get_data_dir.return_value = "/some/random/path" assert backup_info.get_backup_manifest_path() == expected - @patch("barman.infofile.LocalBackupInfo.is_incremental", new_callable=PropertyMock) + @patch("barman.infofile.BackupInfo.is_incremental", new_callable=PropertyMock) def test_get_parent_backup_info_no_parent(self, mock_is_incremental, backup_info): """ Ensure :meth:`LocalBackupInfo.get_parent_backup_info` returns ``None`` if the @@ -956,7 +974,7 @@ def test_get_parent_backup_info_no_parent(self, mock_is_incremental, backup_info mock_is_incremental.return_value = False assert backup_info.get_parent_backup_info() is None - @patch("barman.infofile.LocalBackupInfo.is_incremental", new_callable=PropertyMock) + @patch("barman.infofile.BackupInfo.is_incremental", new_callable=PropertyMock) def test_get_parent_backup_info_empty_parent( self, mock_is_incremental, backup_info ): @@ -972,7 +990,7 @@ def test_get_parent_backup_info_empty_parent( assert backup_info.get_parent_backup_info() is None mock.assert_called_once_with(backup_info.server, backup_id="SOME_ID") - @patch("barman.infofile.LocalBackupInfo.is_incremental", new_callable=PropertyMock) + @patch("barman.infofile.BackupInfo.is_incremental", new_callable=PropertyMock) def test_get_parent_backup_info_parent_ok(self, mock_is_incremental, backup_info): """ Ensure :meth:`LocalBackupInfo.get_parent_backup_info` returns the backup info @@ -1189,7 +1207,7 @@ def provide_child_backup_info(server, backup_id): ("off", "off", "on", "on", False), ), ) - @patch("barman.infofile.LocalBackupInfo.is_incremental", new_callable=PropertyMock) + @patch("barman.infofile.BackupInfo.is_incremental", new_callable=PropertyMock) def test_is_checksum_consistent( self, mock_is_incremental, conf_root, conf_inc1, conf_inc2, conf_inc3, expected ): @@ -1221,7 +1239,7 @@ def test_is_checksum_consistent( walk_mock.return_value = (incremental2, incremental1, root_backup) assert incremental3.is_checksum_consistent() is expected - @patch("barman.infofile.LocalBackupInfo.is_incremental", new_callable=PropertyMock) + @patch("barman.infofile.BackupInfo.is_incremental", new_callable=PropertyMock) def test_true_is_full_and_eligible_for_incremental(self, mock_is_incremental): """ Test that the function applies the correct conditions for a full backup @@ -1256,7 +1274,7 @@ def test_true_is_full_and_eligible_for_incremental(self, mock_is_incremental): ("rsync", "off", False), ], ) - @patch("barman.infofile.LocalBackupInfo.is_incremental", new_callable=PropertyMock) + @patch("barman.infofile.BackupInfo.is_incremental", new_callable=PropertyMock) def test_false_is_full_and_eligible_for_incremental( self, mock_is_incremental, backup_method, summarize_wal, is_incremental ): @@ -1279,24 +1297,6 @@ def test_false_is_full_and_eligible_for_incremental( assert not backup_info.is_full_and_eligible_for_incremental() - @pytest.mark.parametrize( - ("mode", "parent_backup_id", "expected_backup_type"), - [ - ("rsync", None, "rsync"), - ("postgres", "some_id", "incremental"), - ("postgres", None, "full"), - ("snapshot", None, "snapshot"), - ], - ) - def test_backup_type(self, mode, parent_backup_id, expected_backup_type): - """ - Ensure :meth:`LocalBackupInfo.backup_type` returns the correct backup type label. - """ - backup_info = build_test_backup_info(parent_backup_id=parent_backup_id) - backup_info.mode = mode - - assert backup_info.backup_type == expected_backup_type - class TestSyntheticBackupInfo: """