* [pbs-devel] [PATCH proxmox-backup 0/2] Fix DatastoreListSummary on limited permissions @ 2023-12-07 14:35 Gabriel Goller 2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 1/2] ui: datastore summary handle non-existent values Gabriel Goller 2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs Gabriel Goller 0 siblings, 2 replies; 7+ messages in thread From: Gabriel Goller @ 2023-12-07 14:35 UTC (permalink / raw) To: pbs-devel When the user has limited permissions on a datastore (e.g `NoAccess` on the whole datastore and `DatastoreBackup` on a namespace inside the datastore) the Datastore List shows weird values. Instead of returning -1 we return an Option and handle missing values on the frontend. proxmox-backup: Gabriel Goller (2): ui: datastore summary handle non-existent values status: use Option on avail/used datastore attrs pbs-api-types/src/datastore.rs | 19 +++++++++++-------- src/api2/status.rs | 6 +++--- www/datastore/DataStoreListSummary.js | 20 ++++++++++++++------ 3 files changed, 28 insertions(+), 17 deletions(-) Summary over all repositories: 3 files changed, 28 insertions(+), 17 deletions(-) -- murpp v0.4.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 1/2] ui: datastore summary handle non-existent values 2023-12-07 14:35 [pbs-devel] [PATCH proxmox-backup 0/2] Fix DatastoreListSummary on limited permissions Gabriel Goller @ 2023-12-07 14:35 ` Gabriel Goller 2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs Gabriel Goller 1 sibling, 0 replies; 7+ messages in thread From: Gabriel Goller @ 2023-12-07 14:35 UTC (permalink / raw) To: pbs-devel Correctly display missing 'avail' and 'used' attributes in the datatstore summary. This simply sets it to 0, so that we don't get any errors in the console. Signed-off-by: Gabriel Goller <g.goller@proxmox.com> --- www/datastore/DataStoreListSummary.js | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/www/datastore/DataStoreListSummary.js b/www/datastore/DataStoreListSummary.js index 968239b0..b908034d 100644 --- a/www/datastore/DataStoreListSummary.js +++ b/www/datastore/DataStoreListSummary.js @@ -52,12 +52,20 @@ Ext.define('PBS.datastore.DataStoreListSummary', { vm.set('maintenance', ''); } - let total = statusData.avail + statusData.used; - let usage = statusData.used / total; - let usagetext = Ext.String.format(gettext('{0} of {1}'), - Proxmox.Utils.format_size(statusData.used, true), - Proxmox.Utils.format_size(total, true), - ); + let usagetext; + let usage; + + if (Object.hasOwn(statusData, 'avail') && Object.hasOwn(statusData, 'used')) { + let total = statusData.avail + statusData.used; + usage = statusData.used / total; + usagetext = Ext.String.format(gettext('{0} of {1}'), + Proxmox.Utils.format_size(statusData.used, true), + Proxmox.Utils.format_size(total, true), + ); + } else { + usagetext = Ext.String.format(gettext('{0} of {1}'), 0, 0); + usage = 0; + } let usagePanel = me.lookup('usage'); usagePanel.updateValue(usage, usagetext); -- 2.39.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs 2023-12-07 14:35 [pbs-devel] [PATCH proxmox-backup 0/2] Fix DatastoreListSummary on limited permissions Gabriel Goller 2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 1/2] ui: datastore summary handle non-existent values Gabriel Goller @ 2023-12-07 14:35 ` Gabriel Goller 2023-12-07 17:07 ` Dietmar Maurer 2023-12-07 17:33 ` Dietmar Maurer 1 sibling, 2 replies; 7+ messages in thread From: Gabriel Goller @ 2023-12-07 14:35 UTC (permalink / raw) To: pbs-devel Instead of returning -1 if we can't get the attributes, we use an Option which will not be serialized on `None`. Signed-off-by: Gabriel Goller <g.goller@proxmox.com> --- pbs-api-types/src/datastore.rs | 19 +++++++++++-------- src/api2/status.rs | 6 +++--- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs index d4ead1d1..c361dc30 100644 --- a/pbs-api-types/src/datastore.rs +++ b/pbs-api-types/src/datastore.rs @@ -1302,12 +1302,15 @@ pub struct DataStoreStatus { /// Status of a Datastore pub struct DataStoreStatusListItem { pub store: String, - /// The Size of the underlying storage in bytes. (-1 on error) - pub total: i64, - /// The used bytes of the underlying storage. (-1 on error) - pub used: i64, + /// The Size of the underlying storage in bytes. + #[serde(skip_serializing_if = "Option::is_none")] + pub total: Option<i64>, + /// The used bytes of the underlying storage. + #[serde(skip_serializing_if = "Option::is_none")] + pub used: Option<i64>, /// The available bytes of the underlying storage. (-1 on error) - pub avail: i64, + #[serde(skip_serializing_if = "Option::is_none")] + pub avail: Option<i64>, /// A list of usages of the past (last Month). #[serde(skip_serializing_if = "Option::is_none")] pub history: Option<Vec<Option<f64>>>, @@ -1335,9 +1338,9 @@ impl DataStoreStatusListItem { pub fn empty(store: &str, err: Option<String>) -> Self { DataStoreStatusListItem { store: store.to_owned(), - total: -1, - used: -1, - avail: -1, + total: None, + used: None, + avail: None, history: None, history_start: None, history_delta: None, diff --git a/src/api2/status.rs b/src/api2/status.rs index ff7d4cbd..6e758187 100644 --- a/src/api2/status.rs +++ b/src/api2/status.rs @@ -68,9 +68,9 @@ pub async fn datastore_status( let mut entry = DataStoreStatusListItem { store: store.clone(), - total: status.total as i64, - used: status.used as i64, - avail: status.available as i64, + total: Some(status.total as i64), + used: Some(status.used as i64), + avail: Some(status.available as i64), history: None, history_start: None, history_delta: None, -- 2.39.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs 2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs Gabriel Goller @ 2023-12-07 17:07 ` Dietmar Maurer 2023-12-07 17:10 ` Dietmar Maurer 2023-12-07 17:33 ` Dietmar Maurer 1 sibling, 1 reply; 7+ messages in thread From: Dietmar Maurer @ 2023-12-07 17:07 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Gabriel Goller But this is not backwards compatible. Deserialization fail now for -1 (when connecting to an old server which returns -1)? > On 7.12.2023 15:35 CET Gabriel Goller <g.goller@proxmox.com> wrote: > > > Instead of returning -1 if we can't get the attributes, we use an > Option which will not be serialized on `None`. > > Signed-off-by: Gabriel Goller <g.goller@proxmox.com> > --- > pbs-api-types/src/datastore.rs | 19 +++++++++++-------- > src/api2/status.rs | 6 +++--- > 2 files changed, 14 insertions(+), 11 deletions(-) > > diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs > index d4ead1d1..c361dc30 100644 > --- a/pbs-api-types/src/datastore.rs > +++ b/pbs-api-types/src/datastore.rs > @@ -1302,12 +1302,15 @@ pub struct DataStoreStatus { > /// Status of a Datastore > pub struct DataStoreStatusListItem { > pub store: String, > - /// The Size of the underlying storage in bytes. (-1 on error) > - pub total: i64, > - /// The used bytes of the underlying storage. (-1 on error) > - pub used: i64, > + /// The Size of the underlying storage in bytes. > + #[serde(skip_serializing_if = "Option::is_none")] > + pub total: Option<i64>, > + /// The used bytes of the underlying storage. > + #[serde(skip_serializing_if = "Option::is_none")] > + pub used: Option<i64>, > /// The available bytes of the underlying storage. (-1 on error) > - pub avail: i64, > + #[serde(skip_serializing_if = "Option::is_none")] > + pub avail: Option<i64>, > /// A list of usages of the past (last Month). > #[serde(skip_serializing_if = "Option::is_none")] > pub history: Option<Vec<Option<f64>>>, > @@ -1335,9 +1338,9 @@ impl DataStoreStatusListItem { > pub fn empty(store: &str, err: Option<String>) -> Self { > DataStoreStatusListItem { > store: store.to_owned(), > - total: -1, > - used: -1, > - avail: -1, > + total: None, > + used: None, > + avail: None, > history: None, > history_start: None, > history_delta: None, > diff --git a/src/api2/status.rs b/src/api2/status.rs > index ff7d4cbd..6e758187 100644 > --- a/src/api2/status.rs > +++ b/src/api2/status.rs > @@ -68,9 +68,9 @@ pub async fn datastore_status( > > let mut entry = DataStoreStatusListItem { > store: store.clone(), > - total: status.total as i64, > - used: status.used as i64, > - avail: status.available as i64, > + total: Some(status.total as i64), > + used: Some(status.used as i64), > + avail: Some(status.available as i64), > history: None, > history_start: None, > history_delta: None, > -- > 2.39.2 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs 2023-12-07 17:07 ` Dietmar Maurer @ 2023-12-07 17:10 ` Dietmar Maurer 0 siblings, 0 replies; 7+ messages in thread From: Dietmar Maurer @ 2023-12-07 17:10 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Gabriel Goller Oh, Ignore me - I forgot that we do not have any GUI that uses rust for deserialization (it is only my prototype)... > On 7.12.2023 18:07 CET Dietmar Maurer <dietmar@proxmox.com> wrote: > > > But this is not backwards compatible. Deserialization fail now for -1 (when connecting to an old server which returns -1)? ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs 2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs Gabriel Goller 2023-12-07 17:07 ` Dietmar Maurer @ 2023-12-07 17:33 ` Dietmar Maurer 2023-12-11 8:46 ` Gabriel Goller 1 sibling, 1 reply; 7+ messages in thread From: Dietmar Maurer @ 2023-12-07 17:33 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Gabriel Goller But why is it still i64? I would expect u64. > On 7.12.2023 15:35 CET Gabriel Goller <g.goller@proxmox.com> wrote: > > > Instead of returning -1 if we can't get the attributes, we use an > Option which will not be serialized on `None`. > > Signed-off-by: Gabriel Goller <g.goller@proxmox.com> > --- > pbs-api-types/src/datastore.rs | 19 +++++++++++-------- > src/api2/status.rs | 6 +++--- > 2 files changed, 14 insertions(+), 11 deletions(-) > > diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs > index d4ead1d1..c361dc30 100644 > --- a/pbs-api-types/src/datastore.rs > +++ b/pbs-api-types/src/datastore.rs > @@ -1302,12 +1302,15 @@ pub struct DataStoreStatus { > /// Status of a Datastore > pub struct DataStoreStatusListItem { > pub store: String, > - /// The Size of the underlying storage in bytes. (-1 on error) > - pub total: i64, > - /// The used bytes of the underlying storage. (-1 on error) > - pub used: i64, > + /// The Size of the underlying storage in bytes. > + #[serde(skip_serializing_if = "Option::is_none")] > + pub total: Option<i64>, > + /// The used bytes of the underlying storage. > + #[serde(skip_serializing_if = "Option::is_none")] > + pub used: Option<i64>, > /// The available bytes of the underlying storage. (-1 on error) > - pub avail: i64, > + #[serde(skip_serializing_if = "Option::is_none")] > + pub avail: Option<i64>, > /// A list of usages of the past (last Month). > #[serde(skip_serializing_if = "Option::is_none")] > pub history: Option<Vec<Option<f64>>>, > @@ -1335,9 +1338,9 @@ impl DataStoreStatusListItem { > pub fn empty(store: &str, err: Option<String>) -> Self { > DataStoreStatusListItem { > store: store.to_owned(), > - total: -1, > - used: -1, > - avail: -1, > + total: None, > + used: None, > + avail: None, > history: None, > history_start: None, > history_delta: None, > diff --git a/src/api2/status.rs b/src/api2/status.rs > index ff7d4cbd..6e758187 100644 > --- a/src/api2/status.rs > +++ b/src/api2/status.rs > @@ -68,9 +68,9 @@ pub async fn datastore_status( > > let mut entry = DataStoreStatusListItem { > store: store.clone(), > - total: status.total as i64, > - used: status.used as i64, > - avail: status.available as i64, > + total: Some(status.total as i64), > + used: Some(status.used as i64), > + avail: Some(status.available as i64), > history: None, > history_start: None, > history_delta: None, > -- > 2.39.2 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs 2023-12-07 17:33 ` Dietmar Maurer @ 2023-12-11 8:46 ` Gabriel Goller 0 siblings, 0 replies; 7+ messages in thread From: Gabriel Goller @ 2023-12-11 8:46 UTC (permalink / raw) To: Dietmar Maurer, Proxmox Backup Server development discussion On 12/7/23 18:33, Dietmar Maurer wrote: > But why is it still i64? I would expect u64. Yep, that makes more sense :) I'll submit a v2. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-12-11 8:46 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-12-07 14:35 [pbs-devel] [PATCH proxmox-backup 0/2] Fix DatastoreListSummary on limited permissions Gabriel Goller 2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 1/2] ui: datastore summary handle non-existent values Gabriel Goller 2023-12-07 14:35 ` [pbs-devel] [PATCH proxmox-backup 2/2] status: use Option on avail/used datastore attrs Gabriel Goller 2023-12-07 17:07 ` Dietmar Maurer 2023-12-07 17:10 ` Dietmar Maurer 2023-12-07 17:33 ` Dietmar Maurer 2023-12-11 8:46 ` Gabriel Goller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox