Apply suggestions from peer review

Author: christophehenry

doc/generate_bash_completion.py

@@ -42,7 +42,7 @@
 def get_dict_actions(OPTION_SUBTREE, category):
     ACTIONS = [
         action
-        for action in OPTION_SUBTREE[category]["actions"].keys()
+        for action in OPTION_SUBTREE[category].get("actions", {}).keys()
         if not action.startswith("_")
     ]
     ACTIONS_STR = "{}".format(" ".join(ACTIONS))

share/actionsmap.yml

@@ -2117,7 +2117,7 @@ storage:
                         action: store_true
                 info:
                     action_help: Get hard-drive information
-                    api: GET /users/<name>
+                    api: GET /storage/disk/info/<name>
                     arguments:
                         name:
                             help: Name of the hard drive as returned by 'list' command

src/disks.py

@@ -1,9 +1,5 @@
-from logging import getLogger
-
 import dbus
-from _dbus_bindings import PROPERTIES_IFACE
-
-logger = getLogger("yunohost.storage")
+from yunohost.utils.system import binary_to_human
 
 
 __all__ = ["info", "list"]
@@ -13,30 +9,21 @@
 UDISK_DRIVE_IFC = "org.freedesktop.UDisks2.Drive"
 
 
-def _humaize(byte_size):
-    suffixes = "kMGTPEZYRQ"
-
-    byte_size = float(byte_size)
-
-    if byte_size < 1024:
-        return f"{byte_size}B"
-
-    for i, s in enumerate(suffixes, start=2):
-        unit = 1024**i
-        if byte_size <= unit:
-            return f"{(1024 * (byte_size / unit)):.1f} {s}B"
-
-
-def _query_udisks():
+def _query_udisks() -> list[tuple[str, dict]]:
     bus = dbus.SystemBus()
     manager = dbus.Interface(
         bus.get_object("org.freedesktop.UDisks2", "/org/freedesktop/UDisks2"),
         "org.freedesktop.DBus.ObjectManager",
     )
 
-    for name, dev in manager.GetManagedObjects().items():
-        if name.startswith(UDISK_DRIVE_PATH):
-            yield name.removeprefix(UDISK_DRIVE_PATH), dev[UDISK_DRIVE_IFC]
+    return sorted(
+        (
+            (name.removeprefix(UDISK_DRIVE_PATH), dev[UDISK_DRIVE_IFC])
+            for name, dev in manager.GetManagedObjects().items()
+            if name.startswith(UDISK_DRIVE_PATH)
+        ),
+        key=lambda item: item[1]["SortKey"],
+    )
 
 
 def _disk_infos(name: str, drive: dict, human_readable=False):
@@ -45,7 +32,7 @@ def _disk_infos(name: str, drive: dict, human_readable=False):
         "model": drive["Model"],
         "serial": drive["Serial"],
         "removable": bool(drive["MediaRemovable"]),
-        "size": _humaize(drive["Size"]) if human_readable else drive["Size"],
+        "size": binary_to_human(drive["Size"]) if human_readable else drive["Size"],
     }
 
     if connection_bus := drive["ConnectionBus"]:
@@ -55,7 +42,7 @@ def _disk_infos(name: str, drive: dict, human_readable=False):
         result.update(
             {
                 "type": "HDD",
-                "rpm": "Unknown",
+                "rpm": "Unknown" if human_readable else None,
             }
         )
     elif rotation_rate == 0:
@@ -75,15 +62,12 @@ def list(with_info=False, human_readable=False):
     if not with_info:
         return [name for name, _ in _query_udisks()]
 
-    result = {}
+    result = []
 
     for name, drive in _query_udisks():
-        result[name] = _disk_infos(name, drive, human_readable)
+        result.append(_disk_infos(name, drive, human_readable))
 
-    if human_readable and not result:
-        return "No external media found"
-
-    return result
+    return {"disks": result}
 
 
 def info(name, human_readable=False):
@@ -92,7 +76,7 @@ def info(name, human_readable=False):
         bus.get_object(
             "org.freedesktop.UDisks2", f"/org/freedesktop/UDisks2/drives/{name}"
         ),
-        PROPERTIES_IFACE,
+        dbus.PROPERTIES_IFACE,
     ).GetAll("org.freedesktop.UDisks2.Drive")
 
     return _disk_infos(name, drive, human_readable)

src/storage.py

@@ -1,9 +1,10 @@
-def storage_disk_list(name, human_readable=False):
-    from yunohost.disks import info
-
-    return info(name, human_readable=human_readable)
-
-def storage_disk_info(with_info=False, human_readable=False):
+def storage_disk_list(with_info=False, human_readable=False):
     from yunohost.disks import list
 
     return list(with_info=with_info, human_readable=human_readable)
+
+
+def storage_disk_info(name, human_readable=False):
+    from yunohost.disks import info
+
+    return info(name, human_readable=human_readable)