public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Gabriel Goller <g.goller@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
	Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup v3] close #4723: updated gc view in the ui
Date: Wed, 27 Sep 2023 15:05:54 +0200	[thread overview]
Message-ID: <a7f93a37-f168-3d92-6209-54376d72d386@proxmox.com> (raw)
In-Reply-To: <7d77cb78-4601-4da2-aa15-4b9f61aa401f@proxmox.com>

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




      reply	other threads:[~2023-09-27 13:05 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-20 17:02 Gabriel Goller
2023-09-22  9:36 ` Thomas Lamprecht
2023-09-22 10:45   ` Gabriel Goller
2023-09-22 11:49     ` Thomas Lamprecht
2023-09-27 13:05       ` Gabriel Goller [this message]

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=a7f93a37-f168-3d92-6209-54376d72d386@proxmox.com \
    --to=g.goller@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=t.lamprecht@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