From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id EF0E21FF348
	for <inbox@lore.proxmox.com>; Wed, 17 Apr 2024 16:15:33 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 62A90A0D0;
	Wed, 17 Apr 2024 16:15:34 +0200 (CEST)
Message-ID: <fdae22a8-7f9d-4a4e-a812-e06711f39ac5@proxmox.com>
Date: Wed, 17 Apr 2024 16:15:30 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>, Gabriel Goller <g.goller@proxmox.com>
References: <20231207125126.201502-1-g.goller@proxmox.com>
Content-Language: en-US, de-DE
From: Christian Ebner <c.ebner@proxmox.com>
In-Reply-To: <20231207125126.201502-1-g.goller@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.031 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
 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 v4 proxmox{,
 -backup} 0/2] close #4763: client: added command to forget backup
 group
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>

Hi,
thanks for tackling this issue.

The group forget command is something I missed quite a lot when doing 
testing on PBS with accumulating snapshots in a group an not wanting to 
fallback to the UI (I actually always went the route of deleting the 
snapshot folder).

Tested the following:
- Created a few backup snapshots, creating thereby a new backup group
- Tried to delete the group while backup is running
   result was as expected: delete failed, missing lock error
- Tried to delete the group while a restore is running
   result was as expected: delete failed, missing lock (although 
different from the previous one)
- Tried to remove without any other task running
   result was as expected: group deleted, including the group folder in 
the datastore
- Tried to delete empty group
   result was as expected: group deleted, including the group folder in 
the datastore

Two further things I noticed:

The confirmation dialog also allows me to type in `yolo` or any string 
starting with `yY` and nevertheless accepts this as valid confirmation 
input (similar for negation). Should we limit this further?

When one tries to delete a non existing group, the dialog asks me for 
confirmation, failing however afterwards with an error message, leaking 
also the datastore path to the client. While the former is not an issue 
and the intention is to be able to remove empty groups, the latter is 
not okay in my opinion.
So either check if the group even exists before asking for confirmation, 
or map the error to not leak the datastore path.

For the rest consider this:

Tested-by: Christian Ebner <c.ebner@proxmox.com>



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel