From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ; 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 ; 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 ; 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 ; 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 , Proxmox VE development discussion References: <20220304115218.665615-1-m.heiserer@proxmox.com> <20220304115218.665615-3-m.heiserer@proxmox.com> <9b872072-c6f0-3110-e532-b7225e8db2cb@proxmox.com> From: Fabian Ebner 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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!