* [pdm-devel] [PATCH datacenter-manager] server: fake remotes: adapt to changed ClientFactory trait
@ 2025-05-13 13:04 Dominik Csapak
2025-05-13 13:25 ` Lukas Wagner
0 siblings, 1 reply; 5+ messages in thread
From: Dominik Csapak @ 2025-05-13 13:04 UTC (permalink / raw)
To: pdm-devel
that trait changed some types, so we have to adapt here, namely
* use the crate::connection::PveClient type instead of typing it manually
* use Arc instead of Box
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
server/src/test_support/fake_remote.rs | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/server/src/test_support/fake_remote.rs b/server/src/test_support/fake_remote.rs
index 0161d8e..c52bc32 100644
--- a/server/src/test_support/fake_remote.rs
+++ b/server/src/test_support/fake_remote.rs
@@ -1,4 +1,4 @@
-use std::{collections::HashMap, time::Duration};
+use std::{collections::HashMap, sync::Arc, time::Duration};
use anyhow::{bail, Error};
use pdm_api_types::{remotes::Remote, Authid, ConfigDigest};
@@ -6,13 +6,16 @@ use pdm_config::remotes::RemoteConfig;
use proxmox_product_config::ApiLockGuard;
use proxmox_section_config::typed::SectionConfigData;
use pve_api_types::{
- client::PveClient, ClusterMetrics, ClusterMetricsData, ClusterNodeIndexResponse,
- ClusterNodeIndexResponseStatus, ClusterResource, ClusterResourceKind, ClusterResourceType,
- ListTasks, ListTasksResponse, PveUpid, StorageContent,
+ ClusterMetrics, ClusterMetricsData, ClusterNodeIndexResponse, ClusterNodeIndexResponseStatus,
+ ClusterResource, ClusterResourceKind, ClusterResourceType, ListTasks, ListTasksResponse,
+ PveUpid, StorageContent,
};
use serde::Deserialize;
-use crate::{connection::ClientFactory, pbs_client::PbsClient};
+use crate::{
+ connection::{ClientFactory, PveClient},
+ pbs_client::PbsClient,
+};
#[derive(Deserialize, Clone)]
#[serde(rename_all = "kebab-case")]
@@ -74,8 +77,8 @@ impl FakeRemoteConfig {
#[async_trait::async_trait]
impl ClientFactory for FakeClientFactory {
- fn make_pve_client(&self, _remote: &Remote) -> Result<Box<dyn PveClient + Send + Sync>, Error> {
- Ok(Box::new(FakePveClient {
+ fn make_pve_client(&self, _remote: &Remote) -> Result<Arc<PveClient>, Error> {
+ Ok(Arc::new(FakePveClient {
nr_of_vms: self.config.vms_per_pve_remote,
nr_of_cts: self.config.cts_per_pve_remote,
nr_of_nodes: self.config.nodes_per_pve_remote,
@@ -88,7 +91,7 @@ impl ClientFactory for FakeClientFactory {
&self,
_remote: &Remote,
_target_endpoint: Option<&str>,
- ) -> Result<Box<dyn PveClient + Send + Sync>, Error> {
+ ) -> Result<Arc<PveClient>, Error> {
bail!("not implemented")
}
@@ -96,10 +99,7 @@ impl ClientFactory for FakeClientFactory {
bail!("not implemented")
}
- async fn make_pve_client_and_login(
- &self,
- _remote: &Remote,
- ) -> Result<Box<dyn PveClient + Send + Sync>, Error> {
+ async fn make_pve_client_and_login(&self, _remote: &Remote) -> Result<Arc<PveClient>, Error> {
bail!("not implemented")
}
@@ -118,7 +118,7 @@ struct FakePveClient {
}
#[async_trait::async_trait]
-impl PveClient for FakePveClient {
+impl pve_api_types::client::PveClient for FakePveClient {
async fn cluster_resources(
&self,
_ty: Option<ClusterResourceKind>,
--
2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [pdm-devel] [PATCH datacenter-manager] server: fake remotes: adapt to changed ClientFactory trait
2025-05-13 13:04 [pdm-devel] [PATCH datacenter-manager] server: fake remotes: adapt to changed ClientFactory trait Dominik Csapak
@ 2025-05-13 13:25 ` Lukas Wagner
2025-05-14 7:30 ` Dominik Csapak
0 siblings, 1 reply; 5+ messages in thread
From: Lukas Wagner @ 2025-05-13 13:25 UTC (permalink / raw)
To: Proxmox Datacenter Manager development discussion, Dominik Csapak
For the record, I already sent a patch for this in a previous patch series.
https://lore.proxmox.com/pdm-devel/20250418083210.74982-6-l.wagner@proxmox.com/T/#u
On 2025-05-13 15:04, Dominik Csapak wrote:
> that trait changed some types, so we have to adapt here, namely
> * use the crate::connection::PveClient type instead of typing it manually
> * use Arc instead of Box
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> server/src/test_support/fake_remote.rs | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/server/src/test_support/fake_remote.rs b/server/src/test_support/fake_remote.rs
> index 0161d8e..c52bc32 100644
> --- a/server/src/test_support/fake_remote.rs
> +++ b/server/src/test_support/fake_remote.rs
> @@ -1,4 +1,4 @@
> -use std::{collections::HashMap, time::Duration};
> +use std::{collections::HashMap, sync::Arc, time::Duration};
>
> use anyhow::{bail, Error};
> use pdm_api_types::{remotes::Remote, Authid, ConfigDigest};
> @@ -6,13 +6,16 @@ use pdm_config::remotes::RemoteConfig;
> use proxmox_product_config::ApiLockGuard;
> use proxmox_section_config::typed::SectionConfigData;
> use pve_api_types::{
> - client::PveClient, ClusterMetrics, ClusterMetricsData, ClusterNodeIndexResponse,
> - ClusterNodeIndexResponseStatus, ClusterResource, ClusterResourceKind, ClusterResourceType,
> - ListTasks, ListTasksResponse, PveUpid, StorageContent,
> + ClusterMetrics, ClusterMetricsData, ClusterNodeIndexResponse, ClusterNodeIndexResponseStatus,
> + ClusterResource, ClusterResourceKind, ClusterResourceType, ListTasks, ListTasksResponse,
> + PveUpid, StorageContent,
> };
> use serde::Deserialize;
>
> -use crate::{connection::ClientFactory, pbs_client::PbsClient};
> +use crate::{
> + connection::{ClientFactory, PveClient},
> + pbs_client::PbsClient,
> +};
>
> #[derive(Deserialize, Clone)]
> #[serde(rename_all = "kebab-case")]
> @@ -74,8 +77,8 @@ impl FakeRemoteConfig {
>
> #[async_trait::async_trait]
> impl ClientFactory for FakeClientFactory {
> - fn make_pve_client(&self, _remote: &Remote) -> Result<Box<dyn PveClient + Send + Sync>, Error> {
> - Ok(Box::new(FakePveClient {
> + fn make_pve_client(&self, _remote: &Remote) -> Result<Arc<PveClient>, Error> {
> + Ok(Arc::new(FakePveClient {
> nr_of_vms: self.config.vms_per_pve_remote,
> nr_of_cts: self.config.cts_per_pve_remote,
> nr_of_nodes: self.config.nodes_per_pve_remote,
> @@ -88,7 +91,7 @@ impl ClientFactory for FakeClientFactory {
> &self,
> _remote: &Remote,
> _target_endpoint: Option<&str>,
> - ) -> Result<Box<dyn PveClient + Send + Sync>, Error> {
> + ) -> Result<Arc<PveClient>, Error> {
> bail!("not implemented")
> }
>
> @@ -96,10 +99,7 @@ impl ClientFactory for FakeClientFactory {
> bail!("not implemented")
> }
>
> - async fn make_pve_client_and_login(
> - &self,
> - _remote: &Remote,
> - ) -> Result<Box<dyn PveClient + Send + Sync>, Error> {
> + async fn make_pve_client_and_login(&self, _remote: &Remote) -> Result<Arc<PveClient>, Error> {
> bail!("not implemented")
> }
>
> @@ -118,7 +118,7 @@ struct FakePveClient {
> }
>
> #[async_trait::async_trait]
> -impl PveClient for FakePveClient {
> +impl pve_api_types::client::PveClient for FakePveClient {
> async fn cluster_resources(
> &self,
> _ty: Option<ClusterResourceKind>,
--
- Lukas
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [pdm-devel] [PATCH datacenter-manager] server: fake remotes: adapt to changed ClientFactory trait
2025-05-13 13:25 ` Lukas Wagner
@ 2025-05-14 7:30 ` Dominik Csapak
2025-05-14 7:49 ` Lukas Wagner
0 siblings, 1 reply; 5+ messages in thread
From: Dominik Csapak @ 2025-05-14 7:30 UTC (permalink / raw)
To: Lukas Wagner, Proxmox Datacenter Manager development discussion
On 5/13/25 15:25, Lukas Wagner wrote:
> For the record, I already sent a patch for this in a previous patch series.
>
> https://lore.proxmox.com/pdm-devel/20250418083210.74982-6-l.wagner@proxmox.com/T/#u
>
yes, i noticed just before you sent this mail (see my other response, that weirdly
only contains a html part....)
but good to see that we basically have sent the same diff ^^
also, would it make sense to extend the fake remote part?
3 improvements i noticed that would be nice imho
* simulate more api calls
* fix some values (e.g. maxdisk is smaller than disk for hosts)
* add some pseudo random dynamic values? so that it changes over time a bit (e.g. via a configurable
jitter percentage)
this would be nice to fully test the ui without having to setup multiple test clusters
> On 2025-05-13 15:04, Dominik Csapak wrote:
>> that trait changed some types, so we have to adapt here, namely
>> * use the crate::connection::PveClient type instead of typing it manually
>> * use Arc instead of Box
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>> server/src/test_support/fake_remote.rs | 26 +++++++++++++-------------
>> 1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/server/src/test_support/fake_remote.rs b/server/src/test_support/fake_remote.rs
>> index 0161d8e..c52bc32 100644
>> --- a/server/src/test_support/fake_remote.rs
>> +++ b/server/src/test_support/fake_remote.rs
>> @@ -1,4 +1,4 @@
>> -use std::{collections::HashMap, time::Duration};
>> +use std::{collections::HashMap, sync::Arc, time::Duration};
>>
>> use anyhow::{bail, Error};
>> use pdm_api_types::{remotes::Remote, Authid, ConfigDigest};
>> @@ -6,13 +6,16 @@ use pdm_config::remotes::RemoteConfig;
>> use proxmox_product_config::ApiLockGuard;
>> use proxmox_section_config::typed::SectionConfigData;
>> use pve_api_types::{
>> - client::PveClient, ClusterMetrics, ClusterMetricsData, ClusterNodeIndexResponse,
>> - ClusterNodeIndexResponseStatus, ClusterResource, ClusterResourceKind, ClusterResourceType,
>> - ListTasks, ListTasksResponse, PveUpid, StorageContent,
>> + ClusterMetrics, ClusterMetricsData, ClusterNodeIndexResponse, ClusterNodeIndexResponseStatus,
>> + ClusterResource, ClusterResourceKind, ClusterResourceType, ListTasks, ListTasksResponse,
>> + PveUpid, StorageContent,
>> };
>> use serde::Deserialize;
>>
>> -use crate::{connection::ClientFactory, pbs_client::PbsClient};
>> +use crate::{
>> + connection::{ClientFactory, PveClient},
>> + pbs_client::PbsClient,
>> +};
>>
>> #[derive(Deserialize, Clone)]
>> #[serde(rename_all = "kebab-case")]
>> @@ -74,8 +77,8 @@ impl FakeRemoteConfig {
>>
>> #[async_trait::async_trait]
>> impl ClientFactory for FakeClientFactory {
>> - fn make_pve_client(&self, _remote: &Remote) -> Result<Box<dyn PveClient + Send + Sync>, Error> {
>> - Ok(Box::new(FakePveClient {
>> + fn make_pve_client(&self, _remote: &Remote) -> Result<Arc<PveClient>, Error> {
>> + Ok(Arc::new(FakePveClient {
>> nr_of_vms: self.config.vms_per_pve_remote,
>> nr_of_cts: self.config.cts_per_pve_remote,
>> nr_of_nodes: self.config.nodes_per_pve_remote,
>> @@ -88,7 +91,7 @@ impl ClientFactory for FakeClientFactory {
>> &self,
>> _remote: &Remote,
>> _target_endpoint: Option<&str>,
>> - ) -> Result<Box<dyn PveClient + Send + Sync>, Error> {
>> + ) -> Result<Arc<PveClient>, Error> {
>> bail!("not implemented")
>> }
>>
>> @@ -96,10 +99,7 @@ impl ClientFactory for FakeClientFactory {
>> bail!("not implemented")
>> }
>>
>> - async fn make_pve_client_and_login(
>> - &self,
>> - _remote: &Remote,
>> - ) -> Result<Box<dyn PveClient + Send + Sync>, Error> {
>> + async fn make_pve_client_and_login(&self, _remote: &Remote) -> Result<Arc<PveClient>, Error> {
>> bail!("not implemented")
>> }
>>
>> @@ -118,7 +118,7 @@ struct FakePveClient {
>> }
>>
>> #[async_trait::async_trait]
>> -impl PveClient for FakePveClient {
>> +impl pve_api_types::client::PveClient for FakePveClient {
>> async fn cluster_resources(
>> &self,
>> _ty: Option<ClusterResourceKind>,
>
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [pdm-devel] [PATCH datacenter-manager] server: fake remotes: adapt to changed ClientFactory trait
2025-05-14 7:30 ` Dominik Csapak
@ 2025-05-14 7:49 ` Lukas Wagner
2025-05-14 8:02 ` Dominik Csapak
0 siblings, 1 reply; 5+ messages in thread
From: Lukas Wagner @ 2025-05-14 7:49 UTC (permalink / raw)
To: Dominik Csapak, Proxmox Datacenter Manager development discussion
On 2025-05-14 09:30, Dominik Csapak wrote:
> On 5/13/25 15:25, Lukas Wagner wrote:
>> For the record, I already sent a patch for this in a previous patch series.
>>
>> https://lore.proxmox.com/pdm-devel/20250418083210.74982-6-l.wagner@proxmox.com/T/#u
>>
>
> yes, i noticed just before you sent this mail (see my other response, that weirdly
> only contains a html part....)
>
> but good to see that we basically have sent the same diff ^^
>
> also, would it make sense to extend the fake remote part?
>
> 3 improvements i noticed that would be nice imho
> * simulate more api calls
> * fix some values (e.g. maxdisk is smaller than disk for hosts)
> * add some pseudo random dynamic values? so that it changes over time a bit (e.g. via a configurable jitter percentage)
> this would be nice to fully test the ui without having to setup multiple test clusters
>
Sure, makes sense. Feel free to send some patches, otherwise I can have a go at it
when I have the time. The stuff that I already implemented mostly served my own development
needs, for sure there are things missing for other areas of the system.
Another thing that I would like to add is to have a simple way to switch between
multiple fake "profiles" (e.g. different PDM setups sizes, tiny, medium, huge) so
that it is easier to do performance testing in a standardized way. Already
talked to Thomas about this, he provided me some of the variables for these profiles (e.g.
number of remotes, guests per remote, etc.) - but haven't had the time yet to integrate that.
--
- Lukas
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pdm-devel] [PATCH datacenter-manager] server: fake remotes: adapt to changed ClientFactory trait
2025-05-14 7:49 ` Lukas Wagner
@ 2025-05-14 8:02 ` Dominik Csapak
0 siblings, 0 replies; 5+ messages in thread
From: Dominik Csapak @ 2025-05-14 8:02 UTC (permalink / raw)
To: Lukas Wagner, Proxmox Datacenter Manager development discussion
On 5/14/25 09:49, Lukas Wagner wrote:
> On 2025-05-14 09:30, Dominik Csapak wrote:
>> On 5/13/25 15:25, Lukas Wagner wrote:
>>> For the record, I already sent a patch for this in a previous patch series.
>>>
>>> https://lore.proxmox.com/pdm-devel/20250418083210.74982-6-l.wagner@proxmox.com/T/#u
>>>
>>
>> yes, i noticed just before you sent this mail (see my other response, that weirdly
>> only contains a html part....)
>>
>> but good to see that we basically have sent the same diff ^^
>>
>> also, would it make sense to extend the fake remote part?
>>
>> 3 improvements i noticed that would be nice imho
>> * simulate more api calls
>> * fix some values (e.g. maxdisk is smaller than disk for hosts)
>> * add some pseudo random dynamic values? so that it changes over time a bit (e.g. via a configurable jitter percentage)
>> this would be nice to fully test the ui without having to setup multiple test clusters
>>
>
> Sure, makes sense. Feel free to send some patches, otherwise I can have a go at it
> when I have the time. The stuff that I already implemented mostly served my own development
> needs, for sure there are things missing for other areas of the system.
>
> Another thing that I would like to add is to have a simple way to switch between
> multiple fake "profiles" (e.g. different PDM setups sizes, tiny, medium, huge) so
> that it is easier to do performance testing in a standardized way. Already
> talked to Thomas about this, he provided me some of the variables for these profiles (e.g.
> number of remotes, guests per remote, etc.) - but haven't had the time yet to integrate that.
>
sure, I'll send patches for that. I noticed already that in some areas the UI is
not optimal to use when getting to certain large numbers (mostly layout issues, not performance)
so it'll be good to have for ui testing too
as for different profiles, for the short term it could be enough to check in sample
json files into the git repository together with the documentation.
then it'd be a simply copy & restart
(e.g. i have setup my systemd services to provide the ENV variable)
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-05-14 8:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-05-13 13:04 [pdm-devel] [PATCH datacenter-manager] server: fake remotes: adapt to changed ClientFactory trait Dominik Csapak
2025-05-13 13:25 ` Lukas Wagner
2025-05-14 7:30 ` Dominik Csapak
2025-05-14 7:49 ` Lukas Wagner
2025-05-14 8:02 ` Dominik Csapak
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