From 17b686e8a8781a8d42c34fe252162d8454eb0f58 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Radoslav=20Bod=C3=B3?= <bodik@cesnet.cz>
Date: Tue, 9 Apr 2024 17:55:52 +0200
Subject: [PATCH] rwm: drop _cmd from method names which are not mostly
 commands wrapper

---
 README.md             |  1 -
 rwm.py                | 28 +++++++++----------
 tests/test_default.py | 14 +++++-----
 tests/test_rwm.py     | 62 +++++++++++++++++++++----------------------
 tests/test_storage.py |  2 +-
 5 files changed, 53 insertions(+), 54 deletions(-)

diff --git a/README.md b/README.md
index c06a5ec..befc104 100644
--- a/README.md
+++ b/README.md
@@ -47,7 +47,6 @@ TODO:
   old data from the repository/bucket, this should be discussed (howto threat modeling ?)
 * rgw leaks objects on tests
 
-* rwm drop _cmd from methods which are not other commands wrapper
 * fix microceph start on node reboot
 * drop rclone use-cases
 
diff --git a/rwm.py b/rwm.py
index 3032375..4d1045a 100755
--- a/rwm.py
+++ b/rwm.py
@@ -385,7 +385,7 @@ class RWM:
 
         return self.restic_cmd(cmd_args)
 
-    def backup_cmd(self, name) -> subprocess.CompletedProcess:
+    def backup(self, name) -> subprocess.CompletedProcess:
         """backup command"""
 
         if not self.storage_manager.storage_check_policy(self.config["rwm_restic_bucket"]):
@@ -403,7 +403,7 @@ class RWM:
 
         return backup_proc
 
-    def backup_all_cmd(self) -> int:
+    def backup_all(self) -> int:
         """backup all command"""
 
         stats = {}
@@ -427,7 +427,7 @@ class RWM:
         print(tabulate([item.to_dict() for item in stats.values()], headers="keys", numalign="left"))
         return ret
 
-    def storage_create_cmd(self, bucket_name, target_username) -> int:
+    def storage_create(self, bucket_name, target_username) -> int:
         """storage create command"""
 
         try:
@@ -441,7 +441,7 @@ class RWM:
             return 1
         return 0
 
-    def storage_delete_cmd(self, bucket_name) -> int:
+    def storage_delete(self, bucket_name) -> int:
         """storage delete command"""
 
         try:
@@ -451,7 +451,7 @@ class RWM:
             return 1
         return 0
 
-    def storage_check_policy_cmd(self, bucket_name) -> int:
+    def storage_check_policy(self, bucket_name) -> int:
         """storage check policy command"""
 
         ret, msg = (0, "OK") if self.storage_manager.storage_check_policy(bucket_name) else (1, "FAILED")
@@ -459,7 +459,7 @@ class RWM:
         print(msg)
         return ret
 
-    def storage_list_cmd(self):
+    def storage_list(self) -> int:
         """storage_list command"""
 
         print(tabulate(
@@ -469,7 +469,7 @@ class RWM:
         ))
         return 0
 
-    def storage_drop_versions_cmd(self, bucket_name):
+    def storage_drop_versions(self, bucket_name):
         """storage_drop_versions command"""
 
         return self.storage_manager.storage_drop_versions(bucket_name)
@@ -563,22 +563,22 @@ def main(argv=None):  # pylint: disable=too-many-branches
         ret = wrap_output(rwmi.restic_cmd(args.cmd_args))
 
     if args.command == "backup":
-        ret = rwmi.backup_cmd(args.name).returncode
+        ret = rwmi.backup(args.name).returncode
         logger.info("rwm backup finished with %s (ret %d)", "success" if ret == 0 else "errors", ret)
     if args.command == "backup_all":
-        ret = rwmi.backup_all_cmd()
+        ret = rwmi.backup_all()
         logger.info("rwm backup_all finished with %s (ret %d)", "success" if ret == 0 else "errors", ret)
 
     if args.command == "storage_create":
-        ret = rwmi.storage_create_cmd(args.bucket_name, args.target_username)
+        ret = rwmi.storage_create(args.bucket_name, args.target_username)
     if args.command == "storage_delete":
-        ret = rwmi.storage_delete_cmd(args.bucket_name)
+        ret = rwmi.storage_delete(args.bucket_name)
     if args.command == "storage_check_policy":
-        ret = rwmi.storage_check_policy_cmd(args.bucket_name)
+        ret = rwmi.storage_check_policy(args.bucket_name)
     if args.command == "storage_list":
-        ret = rwmi.storage_list_cmd()
+        ret = rwmi.storage_list()
     if args.command == "storage_drop_versions":
-        ret = rwmi.storage_drop_versions_cmd(args.bucket_name)
+        ret = rwmi.storage_drop_versions(args.bucket_name)
 
     logger.debug("rwm finished with %s (ret %d)", "success" if ret == 0 else "errors", ret)
     return ret
diff --git a/tests/test_default.py b/tests/test_default.py
index 98546ef..59547ff 100644
--- a/tests/test_default.py
+++ b/tests/test_default.py
@@ -43,18 +43,18 @@ def test_main(tmpworkdir: str):  # pylint: disable=unused-argument
     with patch.object(rwm.RWM, "restic_cmd", mock_proc):
         assert rwm_main(["restic", "dummy"]) == 0
 
-    with patch.object(rwm.RWM, "backup_cmd", mock_proc):
+    with patch.object(rwm.RWM, "backup", mock_proc):
         assert rwm_main(["backup", "dummy"]) == 0
-    with patch.object(rwm.RWM, "backup_all_cmd", mock_ok):
+    with patch.object(rwm.RWM, "backup_all", mock_ok):
         assert rwm_main(["backup_all"]) == 0
 
-    with patch.object(rwm.RWM, "storage_create_cmd", mock_ok):
+    with patch.object(rwm.RWM, "storage_create", mock_ok):
         assert rwm_main(["storage_create", "bucket", "user"]) == 0
-    with patch.object(rwm.RWM, "storage_delete_cmd", mock_ok):
+    with patch.object(rwm.RWM, "storage_delete", mock_ok):
         assert rwm_main(["storage_delete", "bucket"]) == 0
-    with patch.object(rwm.RWM, "storage_check_policy_cmd", mock_ok):
+    with patch.object(rwm.RWM, "storage_check_policy", mock_ok):
         assert rwm_main(["storage_check_policy", "bucket"]) == 0
-    with patch.object(rwm.RWM, "storage_list_cmd", mock_ok):
+    with patch.object(rwm.RWM, "storage_list", mock_ok):
         assert rwm_main(["storage_list"]) == 0
-    with patch.object(rwm.RWM, "storage_drop_versions_cmd", mock_ok):
+    with patch.object(rwm.RWM, "storage_drop_versions", mock_ok):
         assert rwm_main(["storage_drop_versions", "bucket"]) == 0
diff --git a/tests/test_rwm.py b/tests/test_rwm.py
index d8c22e5..e1773ca 100644
--- a/tests/test_rwm.py
+++ b/tests/test_rwm.py
@@ -105,8 +105,8 @@ def _restic_list_snapshot_files(trwm, snapshot_id):
     return [x["path"] for x in snapshot_ls if (x["struct_type"] == "node") and (x["type"] == "file")]
 
 
-def test_backup_cmd(tmpworkdir: str, motoserver: str):  # pylint: disable=unused-argument
-    """test backup_cmd command"""
+def test_backup(tmpworkdir: str, motoserver: str):  # pylint: disable=unused-argument
+    """test backup"""
 
     trwm = rwm.RWM({
         "rwm_s3_endpoint_url": motoserver,
@@ -131,7 +131,7 @@ def test_backup_cmd(tmpworkdir: str, motoserver: str):  # pylint: disable=unused
     Path("testdatadir/testfile_to_be_ignored").write_text("dummydata", encoding="utf-8")
 
     assert trwm.restic_cmd(["init"]).returncode == 0
-    assert trwm.backup_cmd("testcfg").returncode == 0
+    assert trwm.backup("testcfg").returncode == 0
 
     snapshots = _restic_list_snapshots(trwm)
     assert len(snapshots) == 1
@@ -139,8 +139,8 @@ def test_backup_cmd(tmpworkdir: str, motoserver: str):  # pylint: disable=unused
     assert "/testdatadir/testdata1.txt" in snapshot_files
 
 
-def test_backup_cmd_excludes(tmpworkdir: str, motoserver: str):  # pylint: disable=unused-argument
-    """test backup command"""
+def test_backup_excludes(tmpworkdir: str, motoserver: str):  # pylint: disable=unused-argument
+    """test backu"""
 
     trwm = rwm.RWM({
         "rwm_s3_endpoint_url": motoserver,
@@ -169,7 +169,7 @@ def test_backup_cmd_excludes(tmpworkdir: str, motoserver: str):  # pylint: disab
     Path("testdatadir/var/proc/data").write_text("dummydata", encoding="utf-8")
 
     assert trwm.restic_cmd(["init"]).returncode == 0
-    assert trwm.backup_cmd("testcfg").returncode == 0
+    assert trwm.backup("testcfg").returncode == 0
 
     snapshots = _restic_list_snapshots(trwm)
     assert len(snapshots) == 1
@@ -182,7 +182,7 @@ def test_backup_cmd_excludes(tmpworkdir: str, motoserver: str):  # pylint: disab
     assert "/testdatadir/var/proc/data" in snapshot_files
 
 
-def test_backup_cmd_error_handling(tmpworkdir: str, motoserver: str):  # pylint: disable=unused-argument
+def test_backup_error_handling(tmpworkdir: str, motoserver: str):  # pylint: disable=unused-argument
     """test backup command err cases"""
 
     rwm_conf = {
@@ -197,17 +197,17 @@ def test_backup_cmd_error_handling(tmpworkdir: str, motoserver: str):  # pylint:
     with (
         patch.object(rwm.RWM, "_restic_backup", mock_fail)
     ):
-        assert rwm.RWM(rwm_conf).backup_cmd("dummycfg").returncode == 11
+        assert rwm.RWM(rwm_conf).backup("dummycfg").returncode == 11
 
     with (
         patch.object(rwm.RWM, "_restic_backup", mock_ok),
         patch.object(rwm.RWM, "_restic_forget_prune", mock_fail)
     ):
-        assert rwm.RWM(rwm_conf).backup_cmd("dummycfg").returncode == 11
+        assert rwm.RWM(rwm_conf).backup("dummycfg").returncode == 11
 
 
-def test_backup_all_cmd(tmpworkdir: str):  # pylint: disable=unused-argument
-    """test backup command err cases"""
+def test_backup_all(tmpworkdir: str):  # pylint: disable=unused-argument
+    """test backup_all"""
 
     rwm_conf = {
         "rwm_backups": {
@@ -220,11 +220,11 @@ def test_backup_all_cmd(tmpworkdir: str):  # pylint: disable=unused-argument
         patch.object(rwm.RWM, "_restic_backup", mock),
         patch.object(rwm.RWM, "_restic_forget_prune", mock)
     ):
-        assert rwm.RWM(rwm_conf).backup_all_cmd() == 0
+        assert rwm.RWM(rwm_conf).backup_all() == 0
 
 
-def test_storage_create_cmd(tmpworkdir: str, microceph: str, radosuser_admin: rwm.StorageManager):  # pylint: disable=unused-argument
-    """test_storage_create_cmd"""
+def test_storage_create(tmpworkdir: str, microceph: str, radosuser_admin: rwm.StorageManager):  # pylint: disable=unused-argument
+    """test_storage_create"""
 
     trwm = rwm.RWM({
         "rwm_s3_endpoint_url": radosuser_admin.url,
@@ -233,13 +233,13 @@ def test_storage_create_cmd(tmpworkdir: str, microceph: str, radosuser_admin: rw
     })
 
     bucket_name = "testbuck"
-    assert trwm.storage_create_cmd(bucket_name, "testnx") == 0
-    assert trwm.storage_create_cmd("!invalid", "testnx") == 1
-    assert trwm.storage_create_cmd(bucket_name, "") == 1
+    assert trwm.storage_create(bucket_name, "testnx") == 0
+    assert trwm.storage_create("!invalid", "testnx") == 1
+    assert trwm.storage_create(bucket_name, "") == 1
 
 
-def test_storage_delete_cmd(tmpworkdir: str, microceph: str, radosuser_admin: rwm.StorageManager):  # pylint: disable=unused-argument
-    """test_storage_create_cmd"""
+def test_storage_delete(tmpworkdir: str, microceph: str, radosuser_admin: rwm.StorageManager):  # pylint: disable=unused-argument
+    """test_storage_delete"""
 
     trwm = rwm.RWM({
         "rwm_s3_endpoint_url": radosuser_admin.url,
@@ -260,44 +260,44 @@ def test_storage_delete_cmd(tmpworkdir: str, microceph: str, radosuser_admin: rw
     bucket = trwm.storage_manager.storage_create(bucket_name, "admin")
     assert len(trwm.storage_manager.list_objects(bucket_name)) == 0
     assert trwm.restic_cmd(["init"]).returncode == 0
-    assert trwm.backup_cmd("testcfg").returncode == 0
+    assert trwm.backup("testcfg").returncode == 0
     assert len(trwm.storage_manager.list_objects(bucket_name)) != 0
 
     object_versions = radosuser_admin.s3.meta.client.list_object_versions(Bucket=bucket.name)
     assert len(object_versions["Versions"]) > 0
     assert len(object_versions["DeleteMarkers"]) > 0
 
-    assert trwm.storage_delete_cmd(bucket_name) == 0
+    assert trwm.storage_delete(bucket_name) == 0
     assert not trwm.storage_manager.bucket_exist(bucket_name)
 
-    assert trwm.storage_delete_cmd(bucket_name) == 1
+    assert trwm.storage_delete(bucket_name) == 1
 
 
-def test_storage_check_policy_cmd(tmpworkdir: str):  # pylint: disable=unused-argument
-    """test storage check policy command"""
+def test_storage_check_policy(tmpworkdir: str):  # pylint: disable=unused-argument
+    """test storage check policy"""
 
     trwm = rwm.RWM({})
 
     mock = Mock(return_value=False)
     with patch.object(rwm.StorageManager, "storage_check_policy", mock):
-        assert trwm.storage_check_policy_cmd("dummy") == 1
+        assert trwm.storage_check_policy("dummy") == 1
 
 
-def test_storage_list_cmd(tmpworkdir: str):  # pylint: disable=unused-argument
-    """test storage check policy command"""
+def test_storage_list(tmpworkdir: str):  # pylint: disable=unused-argument
+    """test storage_list"""
 
     trwm = rwm.RWM({})
 
     mock = Mock(return_value=[])
     with patch.object(rwm.StorageManager, "storage_list", mock):
-        assert trwm.storage_list_cmd() == 0
+        assert trwm.storage_list() == 0
 
 
-def test_storage_drop_versions_cmd(tmpworkdir: str):  # pylint: disable=unused-argument
-    """test storage drop versions command"""
+def test_storage_drop_versions(tmpworkdir: str):  # pylint: disable=unused-argument
+    """test storage drop versions"""
 
     trwm = rwm.RWM({})
 
     mock = Mock(return_value=0)
     with patch.object(rwm.StorageManager, "storage_drop_versions", mock):
-        assert trwm.storage_drop_versions_cmd("dummy") == 0
+        assert trwm.storage_drop_versions("dummy") == 0
diff --git a/tests/test_storage.py b/tests/test_storage.py
index 9e8bcf4..c2cb623 100644
--- a/tests/test_storage.py
+++ b/tests/test_storage.py
@@ -138,7 +138,7 @@ def test_storage_backup_usage(
         }
     })
     assert trwm.restic_cmd(["init"]).returncode == 0
-    assert trwm.backup_cmd("dummy").returncode == 0
+    assert trwm.backup("dummy").returncode == 0
 
     assert radosuser_test1.list_objects(bucket_name)
     assert len(json.loads(trwm.restic_cmd(["snapshots", "--json"]).stdout)) == 1
-- 
GitLab