public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Shannon Sterz <s.sterz@proxmox.com>
To: pdm-devel@lists.proxmox.com
Subject: [pdm-devel] [PATCH datacenter-manager 2/4] tree-wide: make `returns` defintion match the return type of api methods
Date: Fri, 28 Nov 2025 16:38:53 +0100	[thread overview]
Message-ID: <20251128153855.543210-3-s.sterz@proxmox.com> (raw)
In-Reply-To: <20251128153855.543210-1-s.sterz@proxmox.com>

this takes usually one of the following forms:

- an array of the type is defined in the `returns` section, but the
type itself not a list of it is returned in the method.
- the `returns` section has a hand-crafted definition that is
incomplete, even though the type itself could be used directly.
- the type in the `returns` section does not match the actual return
type at all (e.g. the method returns a unit -> no `returns` section
should be specified)

Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
---
 server/src/api/config/access/ad.rs     |  1 -
 server/src/api/config/access/ldap.rs   |  1 -
 server/src/api/config/access/openid.rs |  1 -
 server/src/api/pbs/tasks.rs            |  8 ++++++--
 server/src/api/pve/firewall.rs         | 24 ++++--------------------
 server/src/api/pve/lxc.rs              |  4 ++--
 server/src/api/pve/storage.rs          |  2 +-
 server/src/api/pve/tasks.rs            |  8 +++++++-
 server/src/api/remotes.rs              |  4 +---
 server/src/api/resources.rs            |  8 +-------
 server/src/api/sdn/vnets.rs            |  2 +-
 server/src/api/sdn/zones.rs            |  2 +-
 12 files changed, 24 insertions(+), 41 deletions(-)

diff --git a/server/src/api/config/access/ad.rs b/server/src/api/config/access/ad.rs
index 8167082..b5a3d5a 100644
--- a/server/src/api/config/access/ad.rs
+++ b/server/src/api/config/access/ad.rs
@@ -184,7 +184,6 @@ pub enum DeletableProperty {
             },
         },
     },
-    returns:  { type: AdRealmConfig },
     access: {
         permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
     },
diff --git a/server/src/api/config/access/ldap.rs b/server/src/api/config/access/ldap.rs
index c5e7732..374284f 100644
--- a/server/src/api/config/access/ldap.rs
+++ b/server/src/api/config/access/ldap.rs
@@ -216,7 +216,6 @@ pub enum DeletableProperty {
             },
         },
     },
-    returns:  { type: LdapRealmConfig },
     access: {
         permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
     },
diff --git a/server/src/api/config/access/openid.rs b/server/src/api/config/access/openid.rs
index 1832448..5e1764a 100644
--- a/server/src/api/config/access/openid.rs
+++ b/server/src/api/config/access/openid.rs
@@ -184,7 +184,6 @@ pub enum DeletableProperty {
             },
         },
     },
-    returns:  { type: OpenIdRealmConfig },
     access: {
         permission: &Permission::Privilege(&["access", "domains"], PRIV_REALM_ALLOCATE, false),
     },
diff --git a/server/src/api/pbs/tasks.rs b/server/src/api/pbs/tasks.rs
index a178d81..7e86442 100644
--- a/server/src/api/pbs/tasks.rs
+++ b/server/src/api/pbs/tasks.rs
@@ -39,7 +39,11 @@ const UPID_API_SUBDIRS: SubdirMap = &sorted!([
         // FIXME: fine-grained task filtering?
         permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
     },
-    returns: { type: pve_api_types::TaskStatus },
+    returns: {
+        type: Array,
+        description:"A list of tasks.",
+        items: { type: pbs_api_types::TaskListItem },
+    },
 )]
 /// Get the list of tasks either for a specific node, or query all at once.
 async fn list_tasks(remote: String) -> Result<Vec<pbs_api_types::TaskListItem>, Error> {
@@ -89,7 +93,7 @@ async fn stop_task(remote: String, upid: RemoteUpid) -> Result<(), Error> {
         // FIXME: fine-grained task filtering?
         permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
     },
-    returns: { type: pve_api_types::TaskStatus },
+    returns: { type: pdm_api_types::pbs::TaskStatus },
 )]
 /// Get the status of a task from a Proxmox VE instance.
 pub async fn get_task_status(
diff --git a/server/src/api/pve/firewall.rs b/server/src/api/pve/firewall.rs
index 383b2d8..af11d58 100644
--- a/server/src/api/pve/firewall.rs
+++ b/server/src/api/pve/firewall.rs
@@ -337,11 +337,7 @@ pub async fn pve_firewall_status(
             remote: { schema: REMOTE_ID_SCHEMA },
         },
     },
-    returns: {
-        type: Array,
-        description: "Get firewall options.",
-        items: { type: pve_api_types::ClusterFirewallOptions },
-    },
+    returns: { type: pve_api_types::ClusterFirewallOptions },
     access: {
         permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
     },
@@ -456,11 +452,7 @@ pub async fn cluster_firewall_status(
             },
         },
     },
-    returns: {
-        type: Array,
-        description: "Get firewall options.",
-        items: { type: pve_api_types::NodeFirewallOptions },
-    },
+    returns: { type: pve_api_types::NodeFirewallOptions },
     access: {
         permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
     },
@@ -565,11 +557,7 @@ pub async fn cluster_firewall_rules(
             vmid: { schema: VMID_SCHEMA },
         },
     },
-    returns: {
-        type: Array,
-        description: "Get firewall options.",
-        items: { type: pve_api_types::GuestFirewallOptions },
-    },
+    returns: { type: pve_api_types::GuestFirewallOptions },
     access: {
         permission: &Permission::Privilege(&["resource", "{remote}", "guest", "{vmid}"], PRIV_RESOURCE_AUDIT, false),
     },
@@ -687,11 +675,7 @@ pub async fn node_firewall_rules(
             vmid: { schema: VMID_SCHEMA },
         },
     },
-    returns: {
-        type: Array,
-        description: "Get firewall options.",
-        items: { type: pve_api_types::GuestFirewallOptions },
-    },
+    returns: { type: pve_api_types::GuestFirewallOptions },
     access: {
         permission: &Permission::Privilege(&["resource", "{remote}", "guest", "{vmid}"], PRIV_RESOURCE_AUDIT, false),
     },
diff --git a/server/src/api/pve/lxc.rs b/server/src/api/pve/lxc.rs
index 96c0957..dc95783 100644
--- a/server/src/api/pve/lxc.rs
+++ b/server/src/api/pve/lxc.rs
@@ -61,7 +61,7 @@ const LXC_VM_SUBDIRS: SubdirMap = &sorted!([
     returns: {
         type: Array,
         description: "Get a list of containers.",
-        items: { type: pve_api_types::VmEntry },
+        items: { type: pve_api_types::LxcEntry },
     },
     access: {
         permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
@@ -194,7 +194,7 @@ pub async fn lxc_get_pending(
             vmid: { schema: VMID_SCHEMA },
         },
     },
-    returns: { type: pve_api_types::QemuStatus },
+    returns: { type: pve_api_types::LxcStatus },
     access: {
         permission: &Permission::Privilege(&["resource", "{remote}", "guest", "{vmid}"], PRIV_RESOURCE_AUDIT, false),
     },
diff --git a/server/src/api/pve/storage.rs b/server/src/api/pve/storage.rs
index a5338f4..6979711 100644
--- a/server/src/api/pve/storage.rs
+++ b/server/src/api/pve/storage.rs
@@ -27,7 +27,7 @@ const STORAGE_SUBDIR: SubdirMap = &sorted!([
             storage: { schema: PVE_STORAGE_ID_SCHEMA, },
         },
     },
-    returns: { type: pve_api_types::QemuStatus },
+    returns: { type: pve_api_types::StorageStatus },
     access: {
         permission: &Permission::Privilege(&["resource", "{remote}", "storage", "{storage}"], PRIV_RESOURCE_AUDIT, false),
     },
diff --git a/server/src/api/pve/tasks.rs b/server/src/api/pve/tasks.rs
index 5edce14..378f100 100644
--- a/server/src/api/pve/tasks.rs
+++ b/server/src/api/pve/tasks.rs
@@ -43,7 +43,13 @@ const UPID_API_SUBDIRS: SubdirMap = &sorted!([
         // FIXME: fine-grained task filtering?
         permission: &Permission::Privilege(&["resource", "{remote}"], PRIV_RESOURCE_AUDIT, false),
     },
-    returns: { type: pve_api_types::TaskStatus },
+    returns: {
+        type: Array,
+        description: "A list of tasks.",
+        items: {
+            type: pve_api_types::ListTasksResponse
+        }
+    },
 )]
 /// Get the list of tasks either for a specific node, or query all at once.
 async fn list_tasks(
diff --git a/server/src/api/remotes.rs b/server/src/api/remotes.rs
index bc0c871..76b005d 100644
--- a/server/src/api/remotes.rs
+++ b/server/src/api/remotes.rs
@@ -67,9 +67,7 @@ pub fn get_remote<'a>(
         description: "The list of configured remotes.",
         type: Array,
         items: {
-            description: "Remote entry",
-            type: Object,
-            properties: {},
+            type: Remote,
         },
     },
 )]
diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs
index 7eeb0ce..55056e1 100644
--- a/server/src/api/resources.rs
+++ b/server/src/api/resources.rs
@@ -458,13 +458,7 @@ pub(crate) async fn get_resources_impl(
             },
         }
     },
-    returns: {
-        description: "Array of resources, grouped by remote",
-        type: Array,
-        items: {
-            type: RemoteResources,
-        }
-    },
+    returns: { type: RemoteResources },
 )]
 /// Return the amount of configured/seen resources by type
 pub async fn get_status(
diff --git a/server/src/api/sdn/vnets.rs b/server/src/api/sdn/vnets.rs
index ebd2874..5e14c6a 100644
--- a/server/src/api/sdn/vnets.rs
+++ b/server/src/api/sdn/vnets.rs
@@ -149,7 +149,7 @@ async fn list_vnets(
             },
         },
     },
-    returns: { type: String, description: "Worker UPID" },
+    returns: { schema: pdm_api_types::UPID_SCHEMA },
 )]
 /// Create a VNet across multiple remotes
 async fn create_vnet(
diff --git a/server/src/api/sdn/zones.rs b/server/src/api/sdn/zones.rs
index 2ae3d67..c455279 100644
--- a/server/src/api/sdn/zones.rs
+++ b/server/src/api/sdn/zones.rs
@@ -158,7 +158,7 @@ pub async fn list_zones(
             },
         },
     },
-    returns: { type: String, description: "Worker UPID" },
+    returns: { schema: pdm_api_types::UPID_SCHEMA },
 )]
 /// Create a zone across multiple remotes
 async fn create_zone(
-- 
2.47.3



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel


  parent reply	other threads:[~2025-11-28 15:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-28 15:38 [pdm-devel] [PATCH datacenter-manager 0/4] clean up return statements tree wide Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 1/4] tree-wide: add missing `returns` definitions to api macro Shannon Sterz
2025-11-28 15:38 ` Shannon Sterz [this message]
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 3/4] api: pve: make to `get_storage_rrd_data` return PveStorageDataPoint Shannon Sterz
2025-11-28 15:38 ` [pdm-devel] [PATCH datacenter-manager 4/4] api: pbs: fix description of list namespace endpoint Shannon Sterz
2025-11-28 18:53 ` [pdm-devel] applied: [PATCH datacenter-manager 0/4] clean up return statements tree wide Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251128153855.543210-3-s.sterz@proxmox.com \
    --to=s.sterz@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal