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: [RFC datacenter-manager 1/6] connection: store client factory in an Arc and add public getter
Date: Thu, 29 Jan 2026 14:44:13 +0100	[thread overview]
Message-ID: <20260129134418.307552-3-l.wagner@proxmox.com> (raw)
In-Reply-To: <20260129134418.307552-1-l.wagner@proxmox.com>

This will be useful to truly ensure that everybody is using the same
factory.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 server/src/connection.rs                      | 29 ++++++++++++-------
 server/src/context.rs                         |  4 ++-
 .../src/metric_collection/collection_task.rs  |  2 +-
 3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/server/src/connection.rs b/server/src/connection.rs
index 7e366719..f144f9cf 100644
--- a/server/src/connection.rs
+++ b/server/src/connection.rs
@@ -26,7 +26,7 @@ use pve_api_types::client::PveClientImpl;
 
 use crate::pbs_client::PbsClient;
 
-static INSTANCE: OnceLock<Box<dyn ClientFactory + Send + Sync>> = OnceLock::new();
+static INSTANCE: OnceLock<Arc<dyn ClientFactory + Send + Sync>> = OnceLock::new();
 
 /// Connection Info returned from [`prepare_connect_client`]
 struct ConnectInfo {
@@ -396,7 +396,8 @@ impl ClientFactory for DefaultClientFactory {
     }
 }
 
-fn instance() -> &'static (dyn ClientFactory + Send + Sync) {
+/// Get a handle to the global client factory (as an reference).
+pub fn client_factory_ref() -> &'static (dyn ClientFactory + Send + Sync) {
     // Not initializing the connection factory instance is
     // entirely in our responsibility and not something we can recover from,
     // so it should be okay to panic in this case.
@@ -406,9 +407,17 @@ fn instance() -> &'static (dyn ClientFactory + Send + Sync) {
         .as_ref()
 }
 
+/// Get a handle to the global client factory (as an Arc).
+pub fn client_factory() -> Arc<dyn ClientFactory + Send + Sync> {
+    // Not initializing the connection factory instance is
+    // entirely in our responsibility and not something we can recover from,
+    // so it should be okay to panic in this case.
+    Arc::clone(INSTANCE.get().expect("client factory instance not set"))
+}
+
 /// Create a new API client for PVE remotes
 pub fn make_pve_client(remote: &Remote) -> Result<Arc<PveClient>, Error> {
-    instance().make_pve_client(remote)
+    client_factory_ref().make_pve_client(remote)
 }
 
 /// Create a new API client for PVE remotes, but for a specific endpoint
@@ -416,21 +425,21 @@ pub fn make_pve_client_with_endpoint(
     remote: &Remote,
     target_endpoint: Option<&str>,
 ) -> Result<Arc<PveClient>, Error> {
-    instance().make_pve_client_with_endpoint(remote, target_endpoint)
+    client_factory_ref().make_pve_client_with_endpoint(remote, target_endpoint)
 }
 
 /// Create a new API client for PVE remotes and try to make it connect to a specific *node*.
 pub fn make_pve_client_with_node(remote: &Remote, node: &str) -> Result<Arc<PveClient>, Error> {
-    instance().make_pve_client_with_node(remote, node)
+    client_factory_ref().make_pve_client_with_node(remote, node)
 }
 
 /// Create a new API client for PBS remotes
 pub fn make_pbs_client(remote: &Remote) -> Result<Box<PbsClient>, Error> {
-    instance().make_pbs_client(remote)
+    client_factory_ref().make_pbs_client(remote)
 }
 
 pub fn make_raw_client(remote: &Remote) -> Result<Box<Client>, Error> {
-    instance().make_raw_client(remote)
+    client_factory_ref().make_raw_client(remote)
 }
 
 /// Create a new API client for PVE remotes.
@@ -443,7 +452,7 @@ pub fn make_raw_client(remote: &Remote) -> Result<Box<Client>, Error> {
 ///
 /// Note: currently does not support two factor authentication.
 pub async fn make_pve_client_and_login(remote: &Remote) -> Result<Arc<PveClient>, Error> {
-    instance().make_pve_client_and_login(remote).await
+    client_factory_ref().make_pve_client_and_login(remote).await
 }
 
 /// Create a new API client for PBS remotes.
@@ -456,13 +465,13 @@ pub async fn make_pve_client_and_login(remote: &Remote) -> Result<Arc<PveClient>
 ///
 /// Note: currently does not support two factor authentication.
 pub async fn make_pbs_client_and_login(remote: &Remote) -> Result<Box<PbsClient>, Error> {
-    instance().make_pbs_client_and_login(remote).await
+    client_factory_ref().make_pbs_client_and_login(remote).await
 }
 
 /// Initialize the [`ClientFactory`] instance.
 ///
 /// Will panic if the instance has already been set.
-pub fn init(instance: Box<dyn ClientFactory + Send + Sync>) {
+pub fn init(instance: Arc<dyn ClientFactory + Send + Sync>) {
     if INSTANCE.set(instance).is_err() {
         panic!("connection factory instance already set");
     }
diff --git a/server/src/context.rs b/server/src/context.rs
index c5da0afd..8c841eec 100644
--- a/server/src/context.rs
+++ b/server/src/context.rs
@@ -2,6 +2,8 @@
 //!
 //! Make sure to call `init` *once* when starting up the API server.
 
+use std::sync::Arc;
+
 use anyhow::Error;
 
 use crate::connection;
@@ -10,7 +12,7 @@ use crate::connection;
 #[allow(dead_code)]
 fn default_remote_setup() {
     pdm_config::remotes::init(Box::new(pdm_config::remotes::DefaultRemoteConfig));
-    connection::init(Box::new(connection::DefaultClientFactory));
+    connection::init(Arc::new(connection::DefaultClientFactory));
 }
 
 /// Dependency-inject concrete implementations needed at runtime.
diff --git a/server/src/metric_collection/collection_task.rs b/server/src/metric_collection/collection_task.rs
index cc1a460e..00ab2cc0 100644
--- a/server/src/metric_collection/collection_task.rs
+++ b/server/src/metric_collection/collection_task.rs
@@ -553,7 +553,7 @@ pub(super) mod tests {
             // TODO: the client factory is currently stored in a OnceLock -
             // we can only set it from one test... Ideally we'd like to have the
             // option to set it in every single test if needed - task/thread local?
-            connection::init(Box::new(TestClientFactory { now }));
+            connection::init(Arc::new(TestClientFactory { now }));
         });
 
         now
-- 
2.47.3





  parent reply	other threads:[~2026-01-29 13:44 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-29 13:44 [RFC datacenter-manager/proxmox 0/7] inject application context via rpcenv for easier integration testing Lukas Wagner
2026-01-29 13:44 ` [RFC proxmox 1/1] router: rpc environment: allow to provide a application-specific context handle via rpcenv Lukas Wagner
2026-01-29 13:44 ` Lukas Wagner [this message]
2026-01-29 13:44 ` [RFC datacenter-manager 2/6] parallel fetcher: allow to use custom client factory Lukas Wagner
2026-01-29 13:44 ` [RFC datacenter-manager 3/6] introduce PdmApplication struct and inject it during API server startup Lukas Wagner
2026-01-29 13:44 ` [RFC datacenter-manager 4/6] remote updates: use PdmApplication object to derive paths, permissions and client factory Lukas Wagner
2026-01-29 13:44 ` [RFC datacenter-manager 5/6] tests: add captured responses for integration tests Lukas Wagner
2026-01-29 13:44 ` [RFC datacenter-manager 6/6] tests: add basic integration tests for the remote updates API Lukas Wagner
2026-02-03 11:02 ` [RFC datacenter-manager/proxmox 0/7] inject application context via rpcenv for easier integration testing Robert Obkircher

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=20260129134418.307552-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