On Tue, Jan 16, 2024 at 05:12:37PM +0100, Peter Krempa wrote:
> scripts: Add 'qemu-qmp-replies-tool' script for testing and modifying data for qemucapabilitiestest
s/qmp-//
> + try:
> + for line in fh:
> + jsonstr += line
> +
> + if line == '\n':
> + if command is None:
> + command = json.loads(jsonstr)
> + else:
> + conv.append((command, json.loads(jsonstr)))
> + command = None
> +
> + jsonstr = ''
> +
> + if command is not None and jsonstr != '':
> + conv.append((command, json.loads(jsonstr)))
> + command = None
> + jsonstr = ''
> +
> + except json.decoder.JSONDecodeError as je:
> + raise qrtException("JSON error:\n'%s'\nwhile processing snippet:\n'%s'" % (je, jsonstr)) from None
I'm not sure this is the most "pythonic" way to use exceptions, but
I'm also not a purist so I don't mind :)
I've never seen the 'raise Foo() from None' pattern though. Can you
explain why it's needed here?
> +# Process the replies file programmatically here.
> +# The 'conv' argument contains the whole conversation as a list of
> +# (command, reply) tuples, where both command and reply are already parsed JSON
> +# and thus represented by native python types (dict, list, etc ...)
> +#
> +# The code below contains a few examples and hints how to use the programatic
> +# processing. Do not forget to use '--regenerate' flag to uptate the output files.
> +#
> +# Beware that this updates the output file which is used as input for any
> +# subsequent re-run of the tool which can re-apply the modification.
> +def modify_replies(conv):
> + return # remove this to enable modifications
> + version = None # filled with a dictionary with 'major', 'minor', 'micro' keys
> +
> + for (cmd, rep) in conv:
> + if cmd['execute'] == 'query-version':
> + version = rep['return']['qemu']
> + break
> +
> + if version is None:
> + raise Exception("'query-version' not found in the .replies file")
> +
> + idx = -1
> + # Find an index of an entry
> + for i in range(len(conv)):
> + (cmd, rep) = conv[i]
> +
> + if cmd['execute'] == 'device-list-properties':
> + idx = i
> +
> + if idx == -1:
> + raise Exception("entry not found")
> +
> + cmd = {'execute': 'device-list-properties',
> + 'arguments': {'typename': 'example-device'}}
> +
> + reply_unsupp = {'error': {'class': 'DeviceNotFound',
> + 'desc': "Device 'example-device' not found"}}
> +
> + reply = json.loads('''
> + {
> + "return": [
> + {
> + "name": "dummy_prop",
> + "type": "str"
> + },
> + {
> + "name": "test",
> + "type": "str"
> + }
> + ]
> + }
> + ''')
> +
> + if version['major'] >= 8 and version['minor'] > 0:
> + conv.insert(idx, (cmd, reply))
> + else:
> + conv.insert(idx, (cmd, reply_unsupp))
The way these examples are all jumbled together makes it IMO a lot
harder to understand what each is supposed to do, or even where one
ends and the next one starts. I suggest you do something like this
instead:
def modify_replies(conv):
return # no changes
def _modify_replies_example1(conv):
# ...
def _modify_replies_example2(conv):
# ...
> +The default mode is validation which checks the following:
> + - each command has a reply and both are valid JSON
> + - numbering of the 'id' field is as expected
> + - the input file has the expected JSON formatting
> +
> +The tool can be also used to programmaticaly modify the '.replies' file by
> +editting the 'modify_replies' method directly in the source, or for
*editing
> +args = parser.parse_args()
> +
> +if not args.replyfile and not args.repliesdir:
> + parser.print_help()
> + exit(1)
This fails if neither repliesfile or repliesdir have been specified,
but will happily proceed if both are present, in which case only the
former will be considered. Please make sure that scenario is also
handled correctly.
> +if args.replyfile:
> + if not process_one(args.replyfile, args):
> + fail = True
> +else:
> + files = Path(args.repliesdir).glob('*.replies')
> +
> + for file in files:
> + if not process_one(str(file), args):
> + fail = True
This can be simplified as
if args.replyfile:
files = [args.replyfile]
else:
files = Path(args.repliesdir).glob('*.replies')
for file in files:
if not process_one(str(file), args):
fail = True
Honestly I would prefer it if the way to make the script process
multiple files was just to list them all on the command line, but I
understand that you need --repliesdir because meson doesn't have a
native way to perform globbing.
--
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
