[pbs-devel] [PATCH proxmox-backup v2] client/pull: log snapshots that are skipped because of time
Dominik Csapak
d.csapak at proxmox.com
Fri Jun 4 16:03:51 CEST 2021
On 6/4/21 15:54, Dietmar Maurer wrote:
>
>> +impl std::fmt::Display for SkipInfo {
>> + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
>> + if self.count > 1 {
>> + write!(
>> + f,
>> + "{} snapshots ({}..{}) that are older than the newest local snapshot",
>> + self.count,
>> + proxmox::tools::time::epoch_to_rfc3339_utc(self.oldest)
>> + .map_err(|_| std::fmt::Error)?,
>> + proxmox::tools::time::epoch_to_rfc3339_utc(self.newest)
>> + .map_err(|_| std::fmt::Error)?,
>> + )
>
> what is the purpose of this complex message (why we want to show self.oldest and self.newest)?
> Its confusing me more than it helps...
was the suggestion from Fabian, and i did like the idea to tell the user
*which* snapshots were skipped (and if we only have one line,
there is not many ways to represent that)
>
>> + } else if self.count == 1 {
>> + write!(
>> + f,
>> + "1 snapshot ({}) that is older than the newest local snapshot",
>> + proxmox::tools::time::epoch_to_rfc3339_utc(self.oldest)
>> + .map_err(|_| std::fmt::Error)?,
>> + )
>
> do we really need this special case?
if we want to keep the info which snapshots are skipped, then
yes imho, otherwise we have line such as
1 snapshots (X..X) that are older than the newest local snapshot
which is grammatically wrong (1 snapshots are older)
and contains redundant info (X..X)
>
>> + } else {
>> + write!(f, "0 snapshots")
>
> Instead, I would avoid to call this function if count is 0 ...
i already avoid it calling below, but i wanted to implement this,
in case we reuse that struct somewhere else
>
>> + }
>> + }
>> +}
>> +
>> pub async fn pull_group(
More information about the pbs-devel
mailing list