From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <t.lamprecht@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)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 8DC0F74E6B for <pbs-devel@lists.proxmox.com>; Tue, 12 Oct 2021 07:33:24 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7CFA927B4E for <pbs-devel@lists.proxmox.com>; Tue, 12 Oct 2021 07:33:24 +0200 (CEST) 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 E0B1127B40 for <pbs-devel@lists.proxmox.com>; Tue, 12 Oct 2021 07:33:23 +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 B69D245C70 for <pbs-devel@lists.proxmox.com>; Tue, 12 Oct 2021 07:33:23 +0200 (CEST) Message-ID: <43a4f5f5-2fd4-30fd-176a-2f95eec01b95@proxmox.com> Date: Tue, 12 Oct 2021 07:33:22 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:94.0) Gecko/20100101 Thunderbird/94.0 Content-Language: en-US To: Proxmox Backup Server development discussion <pbs-devel@lists.proxmox.com>, Dominik Csapak <d.csapak@proxmox.com> References: <20211011121432.1006944-1-d.csapak@proxmox.com> <9594ead4-8077-c604-b1fc-541be829bd67@proxmox.com> <9dcaa7de-c32a-00c0-f8bd-1d801e7e1325@proxmox.com> <ec7117d4-3a7e-ca8d-bb42-cf23ea8011e7@proxmox.com> <2d4963c3-d86c-b720-33db-239134320f43@proxmox.com> <a97c83e5-53f7-8615-bf17-458fa3277fd5@proxmox.com> <90c21c83-35d2-891c-896e-b42ceb038d85@proxmox.com> From: Thomas Lamprecht <t.lamprecht@proxmox.com> In-Reply-To: <90c21c83-35d2-891c-896e-b42ceb038d85@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.206 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 Subject: Re: [pbs-devel] [PATCH proxmox-backup v2 1/2] rest-server/worker-task: replace newlines with '\n' in task result 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: Tue, 12 Oct 2021 05:33:24 -0000 On 11.10.21 15:57, Dominik Csapak wrote: > On 10/11/21 15:37, Thomas Lamprecht wrote: >>> >> >> I do not mean the root cause that logged a newline at the end, we cannot avoid the future, >> but the one that crates the TaskError enum, and if I'm not completely sleep deprived, that >> is create_state. > > sorry, i misunderstood you, yes, the TaskState::Error gets created in 'create_state', and yes, it makes sense to do it already there... > >> >>> but wanted to have a generic method to not have a wrong task state even >>> when it happens again somewhere >>> >>> imho, percent encoding(%0A) is not nicer in this context here than \n >> >> percent encoding is an actual format than can be rendered again nicely, just randomly >> mapping the \n control code (0x0A) to 0x5C 0x6E, without escaping existing (literal) \n >> just isn't a used format, at least FWICR, that's my point in provoking that question. >> > > while we could decode it again, existing tasks did not get encoded, so we would potentially decode some strings that shouldn't be decoded... > > we could also simply remove the newlines at all, i don't think we would no, same loss of information as replacing it with a literal \n... > benefit from having multiline error messages in tasks.. > since most of the time we want to show a single line per task We want to have the error, if it's longer and over a few lines then that's fine too. > > or what about trying to parse the task log backwards > until we find a correct date+taskerror or at least a correct date? > (in the first case we can assume that the newlines are part of > the error, in the second the state would still be 'unknown' but > with a correct date) brittle to do in general, one cannot guarantee that the error doesn't also contains what we'd match, may not be likely but if we want to improve on the current solution I'd rather go for an actual improvement over an hack.. > > if you don't like either approach, i'd send a > version with percent-encoded/decoded, it's imho more important > that we get the correct task state on task parsing than how we > solve it exactly I'd like to have an actual solution without loss of information, and with our current approach I see only a two-way encoding doing that, I don't care much for what one we use, but good would be as human readable as possible as this is a issue that happens rather rarely. IMO having log + result intertwined into the log isn't ideal either, having a "$UPID.result" (or the like) file with the information already present in a structured format would simplify things a lot..