From: Gabriel Goller <g.goller@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>, Stefan Lendl <s.lendl@proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup v3] close #4723: updated gc view in the ui
Date: Fri, 22 Sep 2023 12:45:33 +0200 [thread overview]
Message-ID: <456bfe52-2a87-e61c-3d2c-ef9b9fccb468@proxmox.com> (raw)
In-Reply-To: <1fbd20c8-2e36-4528-9900-e1ef7dd85499@proxmox.com>
>> Am 20/09/2023 um 19:02 schrieb Gabriel Goller:
>>> Added `ObjectGrid` for GCView, moved to different file. Added
>>> endpoint `gc_info` that returns all the necessary config and
>>> last run stats.
> Sorry to only notice this on v3, but can you please split this patch
> into:
> - new API
> - moving GC view out to own file (with as little semantic change as
> possible)
> - adding new UI
>
> 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?
> [...]
>
>>> + // 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`...
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.
next prev parent reply other threads:[~2023-09-22 10:45 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 [this message]
2023-09-22 11:49 ` Thomas Lamprecht
2023-09-27 13:05 ` Gabriel Goller
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=456bfe52-2a87-e61c-3d2c-ef9b9fccb468@proxmox.com \
--to=g.goller@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
--cc=s.lendl@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