all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
	Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup 3/3] ui: datastore content: add action to show upload statistics
Date: Mon, 27 Nov 2023 13:08:57 +0100	[thread overview]
Message-ID: <992d59dc-8b6c-4c74-9ab0-2d1d5087e2b3@proxmox.com> (raw)
In-Reply-To: <992d0e2d-d3ec-4b2b-be91-79598712c502@proxmox.com>

On 11/27/23 13:02, Thomas Lamprecht wrote:
> On 27.11.23 11:33, Dominik Csapak wrote:
>> On 11/27/23 11:27, Thomas Lamprecht wrote:
>>> Without an in-depth analysis, I think I'd prefer that slightly
>>> more, especially as the maintenance cost of that extra endpoint
>>> should be rather negligible (if there's a good API endpoint path
>>> to put it in, as that sometimes seems to be the harder part ^^)
>>>
>>> And yes, we could then show all the possible data about a
>>> snapshot, even if some of that is currently already included in
>>> the content tree.
>>
>> looking at the code, there really is not much more info about
>> the backups than what we already have in the tree
>> (at least not cheap ones from the manifest etc)
>>
>> the only info we have that is missing from the snapshotlistitem
>> is the group comment, the key fingerprint and the upload statistics,
>> so i'm asking myself if that is really worth a seperate api call...
> 
> Not sure if I'd use the abundance of info in an bloated API call as
> "excuse" to not add a new one, but further bloat the existing one.
> 
> Remember that we want to do a (streaming) API endpoint that returns
> nested objects for the datastore content, where we might want to avoid
> parsing each manifest, for that it might be useful
> 
> It might also be useful for external API users that just want to get the
> info of one snapshot without the huge cost of reading all.
> 
> And it might be also useful for having more options for a potential
> rework of the datastore content UI, e.g., moving comment editing into
> that and some other info or even (lesser used) actions too, that then
> either isn't added to the new endpoint, or one can opt-out for the
> current one.
> 
> Note also that a minimal stats entry , e.g.:
> "upload-statistic":{"count":0,"size":0,"compressed-size":0,"duplicates":0}
> 
> Total to 75 bytes, so for an actual realistic one 100 bytes seems
> reasonable, and while transport compression will help, one still needs
> to have all that in (browser) memory, not a huge cost, but again going
> into the direction we rather want to reverse from.
> 
> Did you thought about the new endpoint with above in mind?  I mean sure,
> above includes a few rather far future looking assumptions, but not sure
> how we ever get away from the current design if we only ever add on top,
> as each specifically checked cost own its own was small (it adds up, on
> multiple levels).

you are absolutely right, bloating the existing one even further is going
into the wrong direction, i'll add a new api endpoint for snapshot
information




  reply	other threads:[~2023-11-27 12:08 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-01  9:29 [pbs-devel] [RFC PATCH proxmox-backup 0/3] show upload statistics in gui Dominik Csapak
2023-08-01  9:29 ` [pbs-devel] [RFC PATCH proxmox-backup 1/3] api-types: make UploadStatistic an api type Dominik Csapak
2023-11-27  9:52   ` Thomas Lamprecht
2023-11-27 10:01     ` Dominik Csapak
2023-11-27 10:12       ` Thomas Lamprecht
2023-11-27 10:17         ` Dominik Csapak
2023-11-27 12:44           ` Wolfgang Bumiller
2023-11-27 14:57             ` Dominik Csapak
2023-08-01  9:29 ` [pbs-devel] [RFC PATCH proxmox-backup 2/3] api: datastore: add upload_statistic to snapshot listing Dominik Csapak
2023-11-27  9:08   ` Wolfgang Bumiller
2023-08-01  9:29 ` [pbs-devel] [PATCH proxmox-backup 3/3] ui: datastore content: add action to show upload statistics Dominik Csapak
2023-11-27  9:28   ` Thomas Lamprecht
2023-11-27 10:04     ` Dominik Csapak
2023-11-27 10:27       ` Thomas Lamprecht
2023-11-27 10:33         ` Dominik Csapak
2023-11-27 12:02           ` Thomas Lamprecht
2023-11-27 12:08             ` Dominik Csapak [this message]
2023-11-27 10:07     ` Thomas Lamprecht
2023-11-27 10:15       ` Dominik Csapak
2023-11-27 12:06         ` 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=992d59dc-8b6c-4c74-9ab0-2d1d5087e2b3@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=t.lamprecht@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