From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 7BBAA60952 for ; Thu, 15 Oct 2020 18:13:44 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6A94F14A1B for ; Thu, 15 Oct 2020 18:13:14 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 4388614A0E for ; Thu, 15 Oct 2020 18:13:13 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 06B8A45DAB for ; Thu, 15 Oct 2020 18:13:13 +0200 (CEST) To: Proxmox Backup Server development discussion , Hannes Laimer References: <20201013093309.14917-1-h.laimer@proxmox.com> <20201013093309.14917-2-h.laimer@proxmox.com> From: Thomas Lamprecht Message-ID: Date: Thu, 15 Oct 2020 18:13:12 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:82.0) Gecko/20100101 Thunderbird/82.0 MIME-Version: 1.0 In-Reply-To: <20201013093309.14917-2-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL -0.132 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [mod.rs] Subject: Re: [pbs-devel] [PATCH v3 proxmox-backup 01/14] add two new schemas for verification jobs X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 15 Oct 2020 16:13:44 -0000 On 13.10.20 11:32, Hannes Laimer wrote: > Signed-off-by: Hannes Laimer > --- > src/api2/types/mod.rs | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) >=20 > diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs > index 75b68879..34468dc3 100644 > --- a/src/api2/types/mod.rs > +++ b/src/api2/types/mod.rs > @@ -302,7 +302,7 @@ pub const PRUNE_SCHEDULE_SCHEMA: Schema =3D StringS= chema::new( > .format(&ApiStringFormat::VerifyFn(crate::tools::systemd::time::ve= rify_calendar_event)) > .schema(); > =20 > -pub const VERIFY_SCHEDULE_SCHEMA: Schema =3D StringSchema::new( this breaks build, VERIFY_SCHEDULE_SCHEMA is still used in config::datast= ore and the respective API module, you only remove that over 4 (!) patches at= the end of the series. No commit should ever break the build. That is a no-go, hard to review and a PITA when doing a bisect. Also, independent of that, I do not really get why this has to be it's own patch - why not squash at least patch 01 to 03, they are effectively one contained change, add a new verification job config with API plumbing= =2E Having patch 04 then separate can be ok, even wanted, it is not related directly to the configuration, but patch 05 could be squashed into 04. patch 06, to 10 is ok as is. The whole removal should be done as single patch at the end. I'd also at least add a snipped to drop the now unrecognized "verify-sche= dule" field, while we do not care for backwards compat, we also do not want to = break all setups which had this set and force them to use the editor to drop th= at line. There are at least two options: 1. a postinst script, similar to what we did for the termproxy change in = the tasks active log, e.g.: # flock -w 30 /etc/proxmox-backup/.datastore.lck sed -i '/^\s\+verify-= schedule /d' /etc/proxmox-backup/datastore.cfg 2. keeping a dummy "verify-schedule", with the schema dropped, which alwa= ys gets deleted when writing out I rather like 1. as it does not makes the real backup code uglier, and it= 's still locked, plus we can use the fact that we can check the old version in pos= tinst and so do this edge triggered only. > +pub const VERIFICATION_SCHEDULE_SCHEMA: Schema =3D StringSchema::new( > "Run verify job at specified schedule.") > .format(&ApiStringFormat::VerifyFn(crate::tools::systemd::time::ve= rify_calendar_event)) > .schema(); > @@ -324,6 +324,16 @@ pub const REMOVE_VANISHED_BACKUPS_SCHEMA: Schema =3D= BooleanSchema::new( > .default(true) > .schema(); > =20 > +pub const IGNORE_VERIFIED_BACKUPS_SCHEMA: Schema =3D BooleanSchema::ne= w( > + "Do not verify backups that are already verified if their verifica= tion is not outdated.") > + .default(true) > + .schema(); > + > +pub const VERIFICATION_OUTDATED_AFTER_SCHEMA: Schema =3D IntegerSchema= ::new( > + "Days after that a verification becomes outdated") > + .minimum(1) > + .schema(); > + > pub const SINGLE_LINE_COMMENT_SCHEMA: Schema =3D StringSchema::new("Co= mment (single line).") > .format(&SINGLE_LINE_COMMENT_FORMAT) > .schema(); >=20