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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 31F45EBC3 for ; Wed, 27 Sep 2023 15:05:56 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 191DBC3E0 for ; Wed, 27 Sep 2023 15:05:56 +0200 (CEST) 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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 27 Sep 2023 15:05:55 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2D56648E37 for ; Wed, 27 Sep 2023 15:05:55 +0200 (CEST) Message-ID: Date: Wed, 27 Sep 2023 15:05:54 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.15.0 To: Thomas Lamprecht , Proxmox Backup Server development discussion References: <20230920170257.311224-1-g.goller@proxmox.com> <1fbd20c8-2e36-4528-9900-e1ef7dd85499@proxmox.com> <456bfe52-2a87-e61c-3d2c-ef9b9fccb468@proxmox.com> <7d77cb78-4601-4da2-aa15-4b9f61aa401f@proxmox.com> Content-Language: en-US From: Gabriel Goller In-Reply-To: <7d77cb78-4601-4da2-aa15-4b9f61aa401f@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.368 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -1.473 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 Subject: Re: [pbs-devel] [PATCH proxmox-backup v3] close #4723: updated gc view in the ui X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Sep 2023 13:05:56 -0000 On 9/22/23 13:49, Thomas Lamprecht wrote: > Am 22/09/2023 um 12:45 schrieb Gabriel Goller: >> Am 22/09/2023 um 11:36 schrieb Thomas Lamprecht: >>> In general, it can be fine to do API and UI in one patch if they are >>> quite coupled, and it's a relatively small and targeted feature. But, if >>> there's existing UI code it still can be nicer to split this so one can >>> differ easier between file-movement changes and actual code changes >>> (git's color-moved isn't IME that helpful) >> So I should submit one patch with the new api endpoint, another one >> with moving the gc view ouf of the file (which I will have to create because >> the PrunceAndGCView makes one api request for both tables and uses the >> same store) and another one for the new ui? > Hmm, forgot that this is using a shared store currently. So yeah, that > make such a split rather busy work.. > I'd then maybe just split UI and API into two separate patches, but not > so hard feelings here. ok >>> [...] >>> >>>>> + // calculate next event >>>>> + if let Some(schedule) = &store_config.gc_schedule { >>>>> + if let (Ok(event), Some(last_run)) = ( >>>>> + schedule.parse::(), >>>>> + computed_schedule.last_run_endtime, >>>>> + ) { >>>>> + if let Ok(next_event) = event.compute_next_event(last_run) { >>>>> + computed_schedule.next_run = next_event; >>>>> + } >>>>> + } >>>>> + } >>>> If GC never ran but is scheduled, it would also be nice to show the next >>>> scheduled run. >>>> This might be usefull if people had not GC configured, then create a job >>>> to further indicate that the job is scheduled correctly. >>>> >>>> >>> I did not check it out closely, but how is the situation handled if it's >>> currently running, returning that under "last_run_upid" would be >>> probably confusing, but without >UPID one cannot allow one to open the >>> log easily, or do we want to have this strictly for last- and next-run? >> Showing the log while the gc is running was never possible (except when you >> manually start it from the ui, then it opens automatically) but we could implement >> a "Show Log" button as we have the `upid`... >> > Yeah, I know that it wasn't possible, but seeing the last state wasn't > either until your patch ;-) > I'm just thinking about what a user might expect to see if there's a > currently running GC operation. > >> When the gc job is currently running, we display a spinner on the status row. To find >> out if the gc job is currently running, we simply check if the `upid` exists and the >> `last_run_endtime` does not. > In the UI of the Proxmox VE replication framework there's also a spinner > if there's currently running a log, but if I press the available "Log" > button there, I then get the task-viewer of the current log, which > has its merits. And, FWIW, GC often runs much longer than a > delta-replication. Implemented a "Show Task Log" button. > FWIW, you could always set the UPID and use the availability of the > "last-run-endtime" property as information for if the task is finished > or still running. > Sent a new patch.