From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EC732C4338F for ; Wed, 18 Aug 2021 18:23:30 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 2684F610FD for ; Wed, 18 Aug 2021 18:23:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 2684F610FD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=nongnu.org Received: from localhost ([::1]:35016 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mGQDo-0002nx-RT for qemu-devel@archiver.kernel.org; Wed, 18 Aug 2021 14:23:28 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:48010) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mGQD2-00026w-Re for qemu-devel@nongnu.org; Wed, 18 Aug 2021 14:22:40 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]:24160) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mGQCy-0002U1-AT for qemu-devel@nongnu.org; Wed, 18 Aug 2021 14:22:39 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1629310954; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=RvD1XfSo5kg4tg0aAGKyJvnDXCUf03qSbB9xkK7ojMs=; b=HxNkYmtfLEQBt/dr2dB0OuPsxEesrC8HPsb9OeUCftbQlQkEyVXmj5dtxDGZEQssaBVDTI 89FyBtbTl9y7sot0OTK88uTN7e4eZECtl9rSv8/jyyDutukCDm1IZQscJMBX5SzJEbGdZc AihrkhgGuLlwxkkcLHkdcZ5zJMio0x0= Received: from mail-ot1-f71.google.com (mail-ot1-f71.google.com [209.85.210.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-420-dGcxAMWVOS-WXxOZv6mUKA-1; Wed, 18 Aug 2021 14:22:32 -0400 X-MC-Unique: dGcxAMWVOS-WXxOZv6mUKA-1 Received: by mail-ot1-f71.google.com with SMTP id 21-20020a9d0595000000b00519f857f5ecso1406230otd.16 for ; Wed, 18 Aug 2021 11:22:32 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=RvD1XfSo5kg4tg0aAGKyJvnDXCUf03qSbB9xkK7ojMs=; b=lc4AyUXkEjc4T7eS/M2ZeDvwQpsPHom85y+Rc0Yd/jVaS7cwVx4VWiy8AYZfmeYTNM yoyo2Yb1rr+qnF04LW9i9cUv35jRhCuRCOUNFpIAhFKijOZRaxXFiPdRLFoWGENyNMGk ByEgjVlSMNj2JsBOvC2Aq/lLExF+CVJoSDu965s5DeFTPzWfLS21WZfuZ8ZtxJ3rXbAk uBHhecvmHG3dhxo//rmgP/k9X1CeXS+Z0UeUs2HN8qg3d7J4nldplb7eL17IOWa7+CbB 5MCb+uTXsJDyWs5VKixSyL/qwkJuTLlW4/DpoMjxixmlAzsgw2/jgbtJQgBz9Mr7vKHA 8f1A== X-Gm-Message-State: AOAM533Myf1exzHVuWtJJJ7konO7dimHwMk3Xbyssu9NdW2WIX8cKWyp XWIqYAofgvRRXZhDkK7lN6J7wMvfznpDQg6ZKPfUwNp/5mIDsTPJsClK32zsqqAg2LjH2UhpYZ8 E/El9aay4iiiW5YfpWpVwHrjhsXmlSqY= X-Received: by 2002:a05:6808:1906:: with SMTP id bf6mr7987648oib.52.1629310951770; Wed, 18 Aug 2021 11:22:31 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxtG87s1rhX8Lk7bIyyRFjx65RBvmKoDE/WJREqeRBRcVtFo6ZmRoWVWoAQcarNs/B+awO2aj8AU2LsiWYapNs= X-Received: by 2002:a05:6808:1906:: with SMTP id bf6mr7987641oib.52.1629310951560; Wed, 18 Aug 2021 11:22:31 -0700 (PDT) MIME-Version: 1.0 References: <20210730201846.5147-1-niteesh.gs@gmail.com> <20210730201846.5147-5-niteesh.gs@gmail.com> In-Reply-To: From: John Snow Date: Wed, 18 Aug 2021 14:22:20 -0400 Message-ID: Subject: Re: [PATCH v3 04/13] python/aqmp-tui: Add AQMP TUI draft To: "Niteesh G. S." Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=jsnow@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="0000000000007f2a9005c9d98449" Received-SPF: pass client-ip=170.10.133.124; envelope-from=jsnow@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -34 X-Spam_score: -3.5 X-Spam_bar: --- X-Spam_report: (-3.5 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.7, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H2=-0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Eduardo Habkost , Kashyap Chamarthy , Markus Armbruster , Wainer Moschetta , qemu-devel , Stefan Hajnoczi , Cleber Rosa , Eric Blake Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --0000000000007f2a9005c9d98449 Content-Type: text/plain; charset="UTF-8" On Fri, Aug 13, 2021 at 11:11 AM Niteesh G. S. wrote: > > On Fri, Aug 6, 2021 at 12:28 AM John Snow wrote: > >> >> On Fri, Jul 30, 2021 at 4:19 PM G S Niteesh Babu >> wrote: >> >>> Added a draft of AQMP TUI. >>> >>> Implements the follwing basic features: >>> 1) Command transmission/reception. >>> 2) Shows events asynchronously. >>> 3) Shows server status in the bottom status bar. >>> >>> Also added necessary pylint, mypy configurations >>> >>> Signed-off-by: G S Niteesh Babu >>> >> ... > + >>> +# Using root logger to enable all loggers under qemu and asyncio >>> +LOGGER = logging.getLogger() >>> >> >> The comment isn't quite true; this is the root logger -- but you go on to >> use it to directly log messages. I don't think you should; use a >> module-level logger. >> >> (1) Create a module-level logger that is named after the current module >> name (e.g. qemu.aqmp.aqmp_tui) and use that for logging messages relating >> to this module: >> LOGGER = logging.getLogger(__name__) >> >> (2) Where you need to address the root logger, just use `root_logger = >> logging.getLogger() ` .... I think the main() function is the only place >> you might need this. >> > The way of logging in the TUI is changed such that, all logging is done > through the root level logger. The handlers and levels are installed to > the root level > logger to allow logging from other libraries to be rerouted to the screen > or file. > We talked about this on IRC, so I'll assume our disagreement in vision here is cleared up ... or at least different :) > + >>> + >>> +def format_json(msg): >>> + """ >>> + Formats given multiline JSON message into a single line message. >>> + Converting into single line is more asthetically pleasing when >>> looking >>> + along with error messages compared to multiline JSON. >>> + """ >>> + # FIXME: Use better formatting mechanism. Might break at more >>> complex JSON >>> + # data. >>> + msg = msg.replace('\n', '') >>> + words = msg.split(' ') >>> + words = [word for word in words if word != ''] >>> + return ' '.join(words) >>> + >>> >> >> You can use the JSON module to do this, >> https://docs.python.org/3/library/json.html#json.dumps >> >> Message._serialize uses this technique to send JSON messages over the >> wire that have no newlines: >> >> https://gitlab.com/jsnow/qemu/-/blob/python-async-qmp-aqmp/python/qemu/aqmp/message.py#L132 >> >> by not specifying an indent and including separators with no spaces, we >> can get a JSON representation with all the space squeezed out. You can add >> spaces and still omit the indent/newlines and so on. >> > > But will this work for invalid JSON messages? As far as I have tried it > doesn't work. > Ah, I see ... Nope, that requires a valid JSON message ... Do we *need* to pretty-print invalid JSON? > + >>> +def main(): >>> + parser = argparse.ArgumentParser(description='AQMP TUI') >>> + parser.add_argument('qmp_server', help='Address of the QMP server' >>> + '< UNIX socket path | TCP addr:port >') >>> + parser.add_argument('--log-file', help='The Log file name') >>> + parser.add_argument('--log-level', default='WARNING', >>> + help='Log level >>> ') >>> + parser.add_argument('--asyncio-debug', action='store_true', >>> + help='Enable debug mode for asyncio loop' >>> + 'Generates lot of output, makes TUI unusable >>> when' >>> + 'logs are logged in the TUI itself.' >>> + 'Use only when logging to a file') >>> >> >> Needs spaces between the lines here so that the output reads correctly. >> > The output renders properly for me. Can you please post a pic if it > doesn't render correctly for you? > No screenshot needed, just try running with '--help'. When you concatenate strings like this: parser.add_argument('--asyncio-debug', action='store_true', help='Enable debug mode for asyncio loop' 'Generates lot of output, makes TUI unusable when' 'logs are logged in the TUI itself.' 'Use only when logging to a file') Python is going to do this: help='Enable debug mode for asyncio loop' + 'Generates lot of output, makes TUI unusable when' + ... so you'll get a string like this: help='Enable debug mode for asyncio loopGenerates lot of output, makes TUI unusable when' + ... --0000000000007f2a9005c9d98449 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Fri, Aug 13, 2021 at 11:11 AM Nite= esh G. S. <niteesh.gs@gmail.com<= /a>> wrote:
<= div dir=3D"ltr">
On Fri, Aug 6, 2021 at 12:28 AM John Snow <jsnow@redhat.com> wrote:

On Fri, Jul 30, 2021 at 4:19 PM G S Niteesh Babu <niteesh.gs@gmail.com> = wrote:
Added a d= raft of AQMP TUI.

Implements the follwing basic features:
1) Command transmission/reception.
2) Shows events asynchronously.
3) Shows server status in the bottom status bar.

Also added necessary pylint, mypy configurations

Signed-off-by: G S Niteesh Babu <niteesh.gs@gmail.com>

...
=C2=A0
+
+# Using root logger to enable all loggers under qemu and asyncio
+LOGGER =3D logging.getLogger()

The com= ment isn't quite true; this is the root logger -- but you go on to use = it to directly log messages. I don't think you should; use a module-lev= el logger.

(1) Create a module-level logger that i= s named after the current module name (e.g. qemu.aqmp.aqmp_tui) and use tha= t for logging messages relating to this module:
LOGGER =3D loggin= g.getLogger(__name__)

(2) Where you need to address the r= oot logger, just use `root_logger =3D logging.getLogger() ` .... I think th= e main() function is the only place you might need this.
<= /blockquote>
Th= e way of logging in the TUI is changed such that, all logging is don= e through the root level logger. The handlers and levels are installed to the root level
logger = to allow logging from other libraries to be rerouted to the screen or file.=

We talked about t= his on IRC, so I'll assume our disagreement in vision here is cleared u= p ... or at least different :)
=C2=A0
+
+
+def format_json(msg):
+=C2=A0 =C2=A0 """
+=C2=A0 =C2=A0 Formats given multiline JSON message into a single line mess= age.
+=C2=A0 =C2=A0 Converting into single line is more asthetically pleasing wh= en looking
+=C2=A0 =C2=A0 along with error messages compared to multiline JSON.
+=C2=A0 =C2=A0 """
+=C2=A0 =C2=A0 # FIXME: Use better formatting mechanism. Might break at mor= e complex JSON
+=C2=A0 =C2=A0 # data.
+=C2=A0 =C2=A0 msg =3D msg.replace('\n', '')
+=C2=A0 =C2=A0 words =3D msg.split(' ')
+=C2=A0 =C2=A0 words =3D [word for word in words if word !=3D ''] +=C2=A0 =C2=A0 return ' '.join(words)
+

You can use the JSON module to do thi= s, https://docs.python.org/3/library/json.html#json.dumps

Message._serialize uses this technique to send JSON m= essages over the wire that have no newlines:

by= not specifying an indent and including separators with no spaces, we can g= et a JSON representation with all the space squeezed out. You can add space= s and still omit the indent/newlines and so on.
=C2=A0
But will this work for invalid JSON messages? As far a= s I have tried it doesn't work.

=
Ah, I see ... Nope, that requires a valid JSON message ... Do we= *need* to pretty-print invalid JSON?
=C2=A0
=
The output renders properly for me. Can you= please post a pic if it doesn't render correctly for you?=C2=A0=

No screenshot needed, ju= st try running with '--help'. When you concatenate strings like thi= s:

parser.add_argument('--asyncio-debug', = action=3D'store_true',
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 help= =3D'Enable debug mode for asyncio loop'
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 '= Generates lot of output, makes TUI unusable when'
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 '= logs are logged in the TUI itself.'
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 '= Use only when logging to a file')
Python is going = to do this:

help=3D'Enable debug mode for asyn= cio loop' + 'Generates lot of output, makes TUI unusable when'= =C2=A0+ ...

so you'll get a string like this:<= /div>

help=3D'Enable debug mode for asyncio loopGene= rates lot of output, makes TUI unusable when'=C2=A0+ ...
=
--0000000000007f2a9005c9d98449--