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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 3FBA36B16D for ; Fri, 18 Mar 2022 07:02:44 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2E1471A1EC for ; Fri, 18 Mar 2022 07:02:14 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 4EBB71A1E0 for ; Fri, 18 Mar 2022 07:02:13 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2265641B19 for ; Fri, 18 Mar 2022 07:02:13 +0100 (CET) Message-ID: <72a44e18-e02f-b692-1f3d-da65ca35f6db@proxmox.com> Date: Fri, 18 Mar 2022 07:02:11 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.2 Content-Language: en-US To: Wolfgang Bumiller Cc: pbs-devel@lists.proxmox.com References: <20220311150755.73338-1-h.laimer@proxmox.com> <20220317084057.i64ce6w32su6vfh5@wobu-vie.proxmox.com> From: Hannes Laimer In-Reply-To: <20220317084057.i64ce6w32su6vfh5@wobu-vie.proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.039 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% 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) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pbs-devel] [PATCH-SERIES] replace print by log macro in libraries 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: Fri, 18 Mar 2022 06:02:44 -0000 Am 17.03.22 um 09:40 schrieb Wolfgang Bumiller: > I like the direction of this. > > On Fri, Mar 11, 2022 at 03:07:45PM +0000, Hannes Laimer wrote: >> This series mostly replaces print with the log macro in libs, it also replaces print >> in binaries where it is used to log stuff and not output the result of a command. >> In the process of replacing prints by log macros a few parameters controlling verbosity >> became obsolete and were removed, other 'verbose' parameters influenced the control >> flow and where therefore kept. > > Where does verbosity affect control flow? That sounds strange. > > And I don't think we should just *drop* `--verbose` parameters. I do > think it would be good to *have* them, either by promoting > previously-verbose output to `log::debug` and making the parameter > affect the filter, or by using a task-local variable we don't need to > hand down through all the function calls, though the latter might be a > bit more involved (given that eg. tokio's LocalKey is not inherited > across `spawn()`...) > Logging output that was behind a `if verbose..` was replaced with a log::debug, I should have mentioned that in the summary. If the `verbose` parameter was used to define the commands result(e.g. proxmox-backup-manager version) it was kept. In [1] we only return if self.verbose is true, looking at it again maybe renaming verbose to foreground(or smth similar) might make sense here... but not sure. >> The whole changes were split up into 7 seperate patches[3-9], this was done >> to aviod one huge patch file and improve readability. Those (maybe also 2) >> should be squashed when applied since they are not necesarilly buildable. >> The reason for that is that in a few places 'verbose' parameters were remove. >> >> A verion bump is also needed since patches 2 (and indirectly 3-10) depend on >> the function added to proxmox-router in patch 1. > > As for the helper... I'm a bit unsure here. > We currently always pass "info", and we use "PBS_LOG" as env var > everywhere except the pxar binary. > > While on the one hand flexibility would be nice... I think we could > also just drop the parameters (or make them `Option`s)? I wanted to keep it as general as possible, since it is in a not PBS specific crate, but Option makes sense for the verbosity level, env var name 'PBS_LOG' outside of PBS seems out of place. (init_cli_logger('LOGLVL', None) might look a little weird though...) [1] https://git.proxmox.com/?p=proxmox-backup.git;a=blob;f=pbs-client/src/pxar/fuse.rs;h=0b90ff2ce36c87b2a7fbd208e581c33700a7e9e1;hb=refs/heads/master#l299