all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Lukas Wagner <l.wagner@proxmox.com>
To: pdm-devel@lists.proxmox.com
Subject: [pdm-devel] [PATCH datacenter-manager 2/8] pdm-api-types: remote upid: make upid field private
Date: Tue, 11 Nov 2025 11:50:53 +0100	[thread overview]
Message-ID: <20251111105059.148997-3-l.wagner@proxmox.com> (raw)
In-Reply-To: <20251111105059.148997-1-l.wagner@proxmox.com>

Shielding members behind a getter is beneficial in most cases. This
makes it easier to change the internal representation of the type
later.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 lib/pdm-api-types/src/remote_upid.rs  | 12 +++++++++++-
 server/src/api/pve/tasks.rs           | 18 +++++++++---------
 server/src/remote_tasks/mod.rs        |  2 +-
 server/src/remote_tasks/task_cache.rs |  2 +-
 server/src/sdn_client.rs              |  2 +-
 ui/src/tasks.rs                       |  4 ++--
 6 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/lib/pdm-api-types/src/remote_upid.rs b/lib/pdm-api-types/src/remote_upid.rs
index bf7c1797..6b0f8461 100644
--- a/lib/pdm-api-types/src/remote_upid.rs
+++ b/lib/pdm-api-types/src/remote_upid.rs
@@ -13,7 +13,7 @@ pub const REMOTE_UPID_SCHEMA: Schema = StringSchema::new("A remote UPID")
 pub struct RemoteUpid {
     remote: String,
     /// This is usually a pve upid, but may also be a pbs upid, they have distinct formats.
-    pub upid: String,
+    upid: String,
 }
 
 impl RemoteUpid {
@@ -24,6 +24,16 @@ impl RemoteUpid {
     pub fn into_remote(self) -> String {
         self.remote
     }
+
+    /// Get the raw UPID.
+    pub fn upid(&self) -> &str {
+        &self.upid
+    }
+
+    /// Get the raw UPID, consuming self.
+    pub fn into_upid(self) -> String {
+        self.upid
+    }
 }
 
 impl ApiType for RemoteUpid {
diff --git a/server/src/api/pve/tasks.rs b/server/src/api/pve/tasks.rs
index 0371fa00..9f04a448 100644
--- a/server/src/api/pve/tasks.rs
+++ b/server/src/api/pve/tasks.rs
@@ -89,13 +89,13 @@ async fn stop_task(remote: String, upid: RemoteUpid) -> Result<(), Error> {
     let pve = get_remote(&remotes, upid.remote())?;
 
     let pve_upid: PveUpid = upid
-        .upid
+        .upid()
         .parse()
-        .map_err(|err| format_err!("invalid upid for PVE: {} - {err}", upid.upid))?;
+        .map_err(|err| format_err!("invalid upid for PVE: {} - {err}", upid.upid()))?;
 
     let pve = connect(pve)?;
 
-    Ok(pve.stop_task(&pve_upid.node, &upid.upid).await?)
+    Ok(pve.stop_task(&pve_upid.node, upid.upid()).await?)
 }
 
 #[api(
@@ -135,14 +135,14 @@ pub async fn get_task_status(
     let pve = get_remote(&remotes, upid.remote())?;
 
     let pve_upid: PveUpid = upid
-        .upid
+        .upid()
         .parse()
-        .map_err(|err| format_err!("invalid upid for PVE: {} - {err}", upid.upid))?;
+        .map_err(|err| format_err!("invalid upid for PVE: {} - {err}", upid.upid()))?;
 
     let pve = connect(pve)?;
 
     loop {
-        let status = pve.get_task_status(&pve_upid.node, &upid.upid).await?;
+        let status = pve.get_task_status(&pve_upid.node, upid.upid()).await?;
         if !wait || !status.is_running() {
             break Ok(status);
         }
@@ -218,14 +218,14 @@ async fn read_task_log(
     let pve = get_remote(&remotes, upid.remote())?;
 
     let pve_upid: PveUpid = upid
-        .upid
+        .upid()
         .parse()
-        .map_err(|err| format_err!("invalid upid for PVE: {} - {err}", upid.upid))?;
+        .map_err(|err| format_err!("invalid upid for PVE: {} - {err}", upid.upid()))?;
 
     let pve = connect(pve)?;
 
     let response = pve
-        .get_task_log(&pve_upid.node, &upid.upid, download, limit, start)
+        .get_task_log(&pve_upid.node, upid.upid(), download, limit, start)
         .await?;
 
     for (key, value) in response.attribs {
diff --git a/server/src/remote_tasks/mod.rs b/server/src/remote_tasks/mod.rs
index c4939742..37e689e0 100644
--- a/server/src/remote_tasks/mod.rs
+++ b/server/src/remote_tasks/mod.rs
@@ -53,7 +53,7 @@ pub async fn get_tasks(
                     }
                 }
                 // TODO: Handle PBS tasks
-                let pve_upid: Result<PveUpid, Error> = task.upid.upid.parse();
+                let pve_upid: Result<PveUpid, Error> = task.upid.upid().parse();
                 match pve_upid {
                     Ok(pve_upid) => Some(TaskListItem {
                         upid: task.upid.to_string(),
diff --git a/server/src/remote_tasks/task_cache.rs b/server/src/remote_tasks/task_cache.rs
index 040266ad..cb3b6687 100644
--- a/server/src/remote_tasks/task_cache.rs
+++ b/server/src/remote_tasks/task_cache.rs
@@ -407,7 +407,7 @@ impl WritableTaskCache {
 
             // TODO:: Handle PBS tasks correctly.
             // TODO: This is awkward, maybe overhaul RemoteUpid type to make this easier
-            match task.upid.upid.parse::<PveUpid>() {
+            match task.upid.upid().parse::<PveUpid>() {
                 Ok(upid) => {
                     let node = &upid.node;
                     let remote = task.upid.remote();
diff --git a/server/src/sdn_client.rs b/server/src/sdn_client.rs
index f88d2a52..6b6bdfa9 100644
--- a/server/src/sdn_client.rs
+++ b/server/src/sdn_client.rs
@@ -334,7 +334,7 @@ impl<C> LockedSdnClients<C> {
         loop {
             tokio::time::sleep(Self::POLLING_INTERVAL).await;
 
-            let status = client.get_task_status(&node, &upid.upid).await?;
+            let status = client.get_task_status(&node, upid.upid()).await?;
 
             if !status.is_running() {
                 if status.finished_successfully() == Some(true) {
diff --git a/ui/src/tasks.rs b/ui/src/tasks.rs
index 4ff29755..41a8648d 100644
--- a/ui/src/tasks.rs
+++ b/ui/src/tasks.rs
@@ -112,9 +112,9 @@ pub fn register_pve_tasks() {
 /// If it's a [`RemoteUpid`], prefixes it with the remote name
 pub fn format_optional_remote_upid(upid: &str, include_remote: bool) -> String {
     if let Ok(remote_upid) = upid.parse::<RemoteUpid>() {
-        let description = match remote_upid.upid.parse::<PveUpid>() {
+        let description = match remote_upid.upid().parse::<PveUpid>() {
             Ok(upid) => format_task_description(&upid.worker_type, upid.worker_id.as_deref()),
-            Err(_) => format_upid(&remote_upid.upid),
+            Err(_) => format_upid(&remote_upid.upid()),
         };
         if include_remote {
             format!("{} - {}", remote_upid.remote(), description)
-- 
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-11 10:50 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-11 10:50 [pdm-devel] [PATCH/RFC datacenter-manager 0/8] add type field to RemoteUpid Lukas Wagner
2025-11-11 10:50 ` [pdm-devel] [PATCH datacenter-manager 1/8] pdm-api-types: move RemoteUpid to its own module Lukas Wagner
2025-11-11 10:50 ` Lukas Wagner [this message]
2025-11-11 10:50 ` [pdm-devel] [PATCH datacenter-manager 3/8] pdm-api-types: remote upid: add missing doc strings Lukas Wagner
2025-11-11 10:50 ` [pdm-devel] [PATCH datacenter-manager 4/8] pdm-api-types: remote upid: add basic tests for RemoteUpid ser/deserialization Lukas Wagner
2025-11-11 10:50 ` [pdm-devel] [RFC datacenter-manager 5/8] pdm-api-types: remote upid: add type field to RemoteUpid Lukas Wagner
2025-11-12 20:00   ` Thomas Lamprecht
2025-11-11 10:50 ` [pdm-devel] [RFC datacenter-manager 6/8] pdm-api-types: remote upid: allow to get native UPID type Lukas Wagner
2025-11-11 10:50 ` [pdm-devel] [RFC datacenter-manager 7/8] ui: remote tasks: use correct base url for PBS tasks Lukas Wagner
2025-11-11 10:50 ` [pdm-devel] [RFC datacenter-manager 8/8] remote task cache: handle PBS tasks correctly Lukas Wagner
2025-11-12 15:50 ` [pdm-devel] [PATCH/RFC datacenter-manager 0/8] add type field to RemoteUpid Shannon Sterz
2025-11-12 20:27 ` [pdm-devel] applied-series: " 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=20251111105059.148997-3-l.wagner@proxmox.com \
    --to=l.wagner@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal