Re: [pbs-devel] [PATCH proxmox-backup v3] close #4723: updated gc view in the ui

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

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.

>> [...]
>>
>>>> +    // calculate next event
>>>> +    if let Some(schedule) = &store_config.gc_schedule {
>>>> +        if let (Ok(event), Some(last_run)) = (
>>>> +            schedule.parse::<CalendarEvent>(),
>>>> +            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.

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.