Re: [PATCH v3 04/13] python/aqmp-tui: Add AQMP TUI draft

[Niteesh G. S.]

On Wed, Aug 18, 2021 at 11:52 PM John Snow <jsnow@redhat.com> wrote:

On Fri, Aug 13, 2021 at 11:11 AM Niteesh G. S. <niteesh.gs@gmail.com> wrote:

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 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 <niteesh.gs@gmail.com>

...

 

+
+# 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 :)

Maybe we'll come back to this on v4 :) 

 

+
+
+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?

IMHO Yes it looks pretty.

[1, true, 3]: QMP message is not a JSON object.

This one looks much better than the below one

[ 1,

       true,
3 ]: QMP message is not a JSON object.

 

+
+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 <CRITICAL|ERROR|WARNING|INFO|DEBUG|>')
+    parser.add_argument('--asyncio-debug', action=""> +                        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="">                     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' + ...

Thanks. Fixed.