From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2.migadu.com ([2001:41d0:700:3204::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms8.migadu.com with LMTPS id mOPlJSk0f2UgawAAkFu2QA (envelope-from ) for ; Sun, 17 Dec 2023 18:47:21 +0100 Received: from aspmx1.migadu.com ([2001:41d0:403:4876::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2.migadu.com with LMTPS id SH6aISk0f2U0cQEAe85BDQ (envelope-from ) for ; Sun, 17 Dec 2023 18:47:21 +0100 X-Envelope-To: larch@yhetil.org Authentication-Results: aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=lease-up.com header.s=2017 header.b="FxhnV/t3"; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org"; dmarc=pass (policy=none) header.from=gnu.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1702835241; h=from:from:sender:sender:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:resent-cc: resent-from:resent-sender:resent-message-id:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post:dkim-signature; bh=YYt31buC56lwLvrzqn9LxOpyvA6g1RRuchLOgeu14KU=; b=lXK8IxSVocE5WqghKqMz6V3fCABasNjH3DIGDfWosvAs9b2Mwn/nC5gYExz/rCGzhG/IJl t456pihI68xySQr7rJLDwIHT0oCiIg9JK0OiZWRN8mFOcVkoQ0PLtbzOxXW8Y0iidRb/vm TCtlcz7p9kpJ3XuQsDLp5g7dJ5zOVS3/7jvHc6a75KIAEKN4X9eM45Z9jUNI7CRrgPgGUF 05grfP6CKR/2O3nS0mK8stKeXi9nwaIB0W9BGdou2y8biShR1y9Sx88d7ZLRaeH5THQ2Ip ts49ABUYfK5dkAaQ/b+YQGxUtyfevcStZHLnKHIT84NAOYOorDe6ch562+iinQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=fail ("headers rsa verify failed") header.d=lease-up.com header.s=2017 header.b="FxhnV/t3"; spf=pass (aspmx1.migadu.com: domain of "guix-patches-bounces+larch=yhetil.org@gnu.org" designates 209.51.188.17 as permitted sender) smtp.mailfrom="guix-patches-bounces+larch=yhetil.org@gnu.org"; dmarc=pass (policy=none) header.from=gnu.org ARC-Seal: i=1; s=key1; d=yhetil.org; t=1702835241; a=rsa-sha256; cv=none; b=qjkX8p8a0waJvJxIkyk6YakPx/6yOfKJTPvU9HLqmAZckeDRJa1bbFcfRlPsktyCqNlTTD fNkrgkdvk/WWSQhkfvyIFzWd7VKWvCT6+yMF3cED9l4TeJdMvWZ9DJM5kF04H7nDtfd50s 1uGQqTeiG6CBaOChXIF7HQVwAcW1dzp5w8JhJtoMxPGPraO76JKReL25zlFI7mS499Rx9c fbSpCnKS+sNvSLORiahY3/4uTLj1SyLpiYntZgoSJPyNVeK1+ibVrRzmORHjg9zniDJIBi xB0yuR+IcIajyml+sE3QYukx05LdAmIX8q8dTJtNkX62799T9qmnH4LXPq+ybA== Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 1A05963623 for ; Sun, 17 Dec 2023 18:47:21 +0100 (CET) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rEvED-0006k1-Qp; Sun, 17 Dec 2023 12:47:01 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rEvEC-0006jo-Nr for guix-patches@gnu.org; Sun, 17 Dec 2023 12:47:00 -0500 Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rEvEC-0005aY-FQ for guix-patches@gnu.org; Sun, 17 Dec 2023 12:47:00 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rEvED-0004TC-PJ for guix-patches@gnu.org; Sun, 17 Dec 2023 12:47:01 -0500 X-Loop: help-debbugs@gnu.org Subject: [bug#67497] [PATCH] Multiple deploy hooks in certbot service Resent-From: Felix Lechner Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Sun, 17 Dec 2023 17:47:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 67497 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Bruno Victal , Arun Isaac Cc: 67497@debbugs.gnu.org Received: via spool by 67497-submit@debbugs.gnu.org id=B67497.170283519817143 (code B ref 67497); Sun, 17 Dec 2023 17:47:01 +0000 Received: (at 67497) by debbugs.gnu.org; 17 Dec 2023 17:46:38 +0000 Received: from localhost ([127.0.0.1]:58997 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rEvDp-0004SQ-Gw for submit@debbugs.gnu.org; Sun, 17 Dec 2023 12:46:37 -0500 Received: from sail-ipv4.us-core.com ([208.82.101.137]:41410) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rEvDn-0004SJ-V6 for 67497@debbugs.gnu.org; Sun, 17 Dec 2023 12:46:36 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; s=2017; bh=YYt31buC56lwLvr zqn9LxOpyvA6g1RRuchLOgeu14KU=; h=date:references:in-reply-to:subject: cc:to:from; d=lease-up.com; b=FxhnV/t3Mv9AkQyA2+JhehqwxAW6QCDQAkbl/+D9 +D/9aRSyygVeoiJG2TLFEBvprlTn0yUvvPoVBVHEeDqOkIuVOB/1dZp7XWZ9aAGaufEQRa 9budARgqi3X2anJ8JjfWYVQQNnSNvxrIZGW1cMBbISZSc6q2YceDHbhyrOYH4= Received: by sail-ipv4.us-core.com (OpenSMTPD) with ESMTPSA id 0d66f2f2 (TLSv1.3:TLS_CHACHA20_POLY1305_SHA256:256:NO); Sun, 17 Dec 2023 17:46:33 +0000 (UTC) In-Reply-To: References: <87zfyzkkt4.fsf@lease-up.com> <874jh6bu8c.fsf@systemreboot.net> Date: Sun, 17 Dec 2023 09:46:32 -0800 Message-ID: <875y0wrabr.fsf@lease-up.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-to: Felix Lechner X-ACL-Warn: , Felix Lechner via Guix-patches From: Felix Lechner via Guix-patches via Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: guix-patches-bounces+larch=yhetil.org@gnu.org X-Migadu-Country: US X-Migadu-Flow: FLOW_IN X-Migadu-Spam-Score: -6.71 X-Spam-Score: -6.71 X-Migadu-Queue-Id: 1A05963623 X-Migadu-Scanner: mx10.migadu.com X-TUID: DWUFJ8mbe6J0 Hi, Thank you both for reviewing this patch! I have to respond to several reviews and will start with this one, because it weighed the heaviest on me. On Sat, Dec 16 2023, Bruno Victal wrote: > As Arun pointed out, I don't think multiple deploy hooks would be > adding value here. Your blanket opposition to this patch is incomprehensible to me from several angles: 1. A meaningful name for a hook near the certificate declaration is more administrator-friendly. Someone who manages several certificates, like my twenty-one certificates [1], can see right away which services are being restarted. 2. Arun's solution requires an extra procedure and makes the configuration file longer without without conveying extra meaning. 3. Anyone parsing the code has to look up the definition of the hook in order to see what it does---and probably also the definition for 'invoke', which is not standard Guile, in the Guix manual. In my view, your code is not easy to read. 4. The bundling into one script brings no economy, because different services generally share no code for their reloading. That was already recognized by Certbot's upstream when the feature for multiple hooks was added. After all, the concerns can also be combined, as you prefer, in Certbot's own hooks, but that was apparently unpopular. 5. As a more serious downside, in your cases changing the combined hook might inadventently reload a certificate for a service does not use it. A grep is required to check where the cmombined hook is being used. An extra step is required, and the propensity for errors rises. 6. In your preferred setups, the most elegant way to provide different hooks is probably '%certbot-hook-1' and 'certbot-hook-2'. Those scripts will then share code---likely to restart a HTTP server---for no good reason! 7. User-friendliness is regarded as a worthwhile goal at another, more popular Linux distribution. [2] 8. Most significantly, your use case isn't affected by this patch! The use of combined hooks, which you prefer, is still possible should this patch be accepted. In summary, I do not understand what motivated you to object to this patch, but I recognize that the opinions of reasonable people can differ. As a side note, I have contributed upstream, but not to the feature we are discussing here. [3] > What would be interesting though is adding service-extensions support > for certbot-service-type. Roughly speaking, two plausible ways to > achieve this would be: > > * Single deploy-hook and ungexp-splicing, i.e.: > > [...] > > * Multiple --deploy-hook =E2=80=A6 behind the scenes (the deploy-hook > field in still accepts only a single hook) While I very much respect Bruno's opinion and guidance on Guix services (and genuinely appreciated this review) I do not understand what those sentences mean. I guess it's shame on me. I can, however, say that I likewise fail to see an advantage in more complexity when my patch does nearly the same thing in three lines. Thank you! Kind regards Felix [1] https://codeberg.org/lechner/system-config/src/commit/b566b08a982a12f89= 6cd6e6666f7849dbac0ce2e/host/wallace-server/operating-system.scm#L1097-L1193 [2] point 4, https://www.debian.org/social_contract.html [3] https://github.com/certbot/certbot/blob/master/AUTHORS.md