From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <h.laimer@proxmox.com>
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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <pbs-devel@lists.proxmox.com>; 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 <w.bumiller@proxmox.com>
Cc: pbs-devel@lists.proxmox.com
References: <20220311150755.73338-1-h.laimer@proxmox.com>
<20220317084057.i64ce6w32su6vfh5@wobu-vie.proxmox.com>
From: Hannes Laimer <h.laimer@proxmox.com>
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
<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>
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