From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <f.ebner@proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by lists.proxmox.com (Postfix) with ESMTPS id 7AEE369243
for <pve-devel@lists.proxmox.com>; Fri, 11 Mar 2022 12:57:13 +0100 (CET)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id 714ED2423E
for <pve-devel@lists.proxmox.com>; Fri, 11 Mar 2022 12:57:13 +0100 (CET)
Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com
[94.136.29.106])
(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
key-exchange X25519 server-signature RSA-PSS (2048 bits))
(No client certificate requested)
by firstgate.proxmox.com (Proxmox) with ESMTPS id C8CE024235
for <pve-devel@lists.proxmox.com>; Fri, 11 Mar 2022 12:57:12 +0100 (CET)
Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1])
by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 9B26C419E6
for <pve-devel@lists.proxmox.com>; Fri, 11 Mar 2022 12:57:12 +0100 (CET)
Message-ID: <9a29fae2-e96c-a3c3-8e03-57ad8b512cf8@proxmox.com>
Date: Fri, 11 Mar 2022 12:57:10 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
Thunderbird/91.6.2
Content-Language: en-US
To: Matthias Heiserer <m.heiserer@proxmox.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>
References: <20220304115218.665615-1-m.heiserer@proxmox.com>
<20220304115218.665615-3-m.heiserer@proxmox.com>
<c950a02b-5222-ed1d-6e08-66d05808306b@proxmox.com>
<9b872072-c6f0-3110-e532-b7225e8db2cb@proxmox.com>
From: Fabian Ebner <f.ebner@proxmox.com>
In-Reply-To: <9b872072-c6f0-3110-e532-b7225e8db2cb@proxmox.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.123 Adjusted score from AWL reputation of From: address
BAYES_00 -1.9 Bayes spam probability is 0 to 1%
KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment
NICE_REPLY_A -0.001 Looks like a legit reply (A)
SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record
SPF_PASS -0.001 SPF: sender matches SPF record
T_SCC_BODY_TEXT_LINE -0.01 -
Subject: Re: [pve-devel] [PATCH manager 3/3] Storage GUI: Rewrite backup
content view as TreePanel.
X-BeenThere: pve-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/>
List-Post: <mailto:pve-devel@lists.proxmox.com>
List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>,
<mailto:pve-devel-request@lists.proxmox.com?subject=subscribe>
X-List-Received-Date: Fri, 11 Mar 2022 11:57:13 -0000
Am 11.03.22 um 12:33 schrieb Matthias Heiserer:
> On 09.03.2022 13:39, Fabian Ebner wrote:
>> Am 04.03.22 um 12:52 schrieb Matthias Heiserer:
> 8< --------->
>> It'd be great if you could also remove the now unused stuff from the
>> ContentView base class.
>>
>> Did you think about a way to re-use this new class for the guest's
>> backup view already?
> Not yet, as I wasn't certain whether there is a need. Will do both.
If it's possible to do without too much overhead, it's certainly nicer
than the current approach of having duplicated code to keep in sync.
>>
>>> +Ext.define('pve-data-store-backups', {
>>
>> Nit: datastore is not typical for PVE, and I'd prefer singular 'backup'.
>>
>>> + extend: 'Ext.data.Model',
>>> + fields: [
>>> + {
>>> + name: 'ctime',
>>> + date: 'date',
>>> + dateFormat: 'timestamp',
>>> + },
>>> + 'format',
>>> + 'volid',
>>> + 'content',
>>> + 'vmid',
>>> + 'size',
>>> + 'protected',
>>> + 'notes',
>>
>> Does not mention 'text' which is assigned later. And ContentView has
>> special handling to not display the full volid. Please also use
>> PVE.Utils.render_storage_content() for that here. Or we might want to
>> switch to using only the date for the leafs? But that needs some
>> consideration for filtering, and renamed backups which we currently
>> support..
>>
> For now, I would stay with displaying the full date and time, as it is
> the simplest solution.
I meant 'date and time' ;) Just not happy with displaying the full volid.
> 8< ---------
>> @Thomas: in PBS it's also ascending by date. Should that be changed or
>> do we switch back in PVE?
>>
>>> + 'backup-time',
>>
>> Isn't it ctime? That said, ContentView has some special handling for the
>> 'vdate' column, so maybe we should continue using that. It seems like
>> the back-end automatically sets the ctime to be the one from the archive
>> name via archive_info() in case it's a standard name, so maybe it
>> doesn't actually matter.
>>
> Yes, should be ctime.
> From what I understand, vdate in ContentView isn't something from the
> API, but just some convoluted way of displaying ctime. Functionally,
> there shouldn't be any differences.
Before pve-storage commit 9c629b3e76a6c70314a385982944fe7ca051947c, the
API didn't return the ctime calculated from the backup name. The code
for the vdate calculation is older. Using ctime from the API should be fine.
>
> 8< ---------
>>
>>> + let latest = c.reduce((l, r) => l.ctime > r.ctime ? l : r);
>>> + let num_verified = c.reduce((l, r) => l + r.verification ===
>>> 'ok', 0);
>>
>> Does adding a boolean work reliably? Feels hacky.
> According to ECMA standard 7.1.4, which I believe is the relevant
> section, it works just intended and converts true to 1 and false to 0.
>
> 8< ---------
>
> Agree with everything else. Thanks!