public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>,
	Dominik Csapak <d.csapak@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:02:11 +0100	[thread overview]
Message-ID: <992d0e2d-d3ec-4b2b-be91-79598712c502@proxmox.com> (raw)
In-Reply-To: <7d00eb4a-bb62-45a7-895b-18484f51f0cf@proxmox.com>

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).




  reply	other threads:[~2023-11-27 12:02 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 [this message]
2023-11-27 12:08             ` Dominik Csapak
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=992d0e2d-d3ec-4b2b-be91-79598712c502@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=d.csapak@proxmox.com \
    --cc=pbs-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 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