Re: [PATCH v11 09/11] qcow2_format.py: collect fields to dump in JSON format

[Vladimir Sementsov-Ogievskiy]

29.07.2020 08:56, Andrey Shinkevich wrote:
> On 28.07.2020 14:09, Vladimir Sementsov-Ogievskiy wrote:
> > 17.07.2020 11:14, Andrey Shinkevich wrote:
> > > As __dict__ is being extended with class members we do not want to
> > > print, add the to_dict() method to classes that returns a dictionary
> > > with desired fields and their values. Extend it in subclass when
> > > necessary to print the final dictionary in the JSON output which
> > > follows.
> > >
> > > Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> > > ---
> > >   tests/qemu-iotests/qcow2_format.py | 38 ++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 38 insertions(+)
> > >
> > > diff --git a/tests/qemu-iotests/qcow2_format.py 
> > > b/tests/qemu-iotests/qcow2_format.py
> > > index 2921a27..19d29b8 100644
> > > --- a/tests/qemu-iotests/qcow2_format.py
> > > +++ b/tests/qemu-iotests/qcow2_format.py
> ...
> > >     class Qcow2BitmapDirEntry(Qcow2Struct):
> > >   @@ -190,6 +198,13 @@ class Qcow2BitmapDirEntry(Qcow2Struct):
> > >           super(Qcow2BitmapDirEntry, self).dump()
> > >           self.bitmap_table.dump()
> > >   +    def to_dict(self):
> > > +        fields_dict = super().to_dict()
> > > +        fields_dict.update(bitmap_table=self.bitmap_table)
> > > +        bmp_name = dict(name=self.name)
> > > +        bme_dict = {**bmp_name, **fields_dict}
> >
> > hmmm... I don't follow, why not simply
> >
> >            fields_dict = super().to_dict()
> >            fields_dict['name'] = self.name
> >            fields_dict['bitmap_table'] = self.bitmap_table
> >            ?
>
>
> The idea is to put the name ahead of the dict. It is the same to 
> QcowHeaderExtension::to_dict(). The relevant comment will be supplied in the 
> code.

Not worth doing. Json is not human output, it's mostly for parsing, so using so 
hard magic in the code to sort fields as you want is not worth doing. And I'm 
not sure how much is it guaranteed to keep some ordering of dict fields, why 
can't it change from version to version?

> The .update() will be replaced with the assignment operator.
>
> Andrey
>
>
> > > +        return bme_dict
> > > +
> ...
> > > @@ -303,6 +327,17 @@ class QcowHeaderExtension(Qcow2Struct):
> > >           else:
> > >               self.obj.dump()
> > >   +    def to_dict(self):
> > > +        fields_dict = super().to_dict()
> > > +        ext_name = dict(name=self.Magic(self.magic))
> > > +        he_dict = {**ext_name, **fields_dict}
> >
> > again, why not just add a field to fields_dict ?
> >
> > > +        if self.obj is not None:
> > > +            he_dict.update(data=self.obj)
> > > +        else:
> > > +            he_dict.update(data_str=self.data_str)
> > > +
> > > +        return he_dict
> > > +
> ...


--
Best regards,
Vladimir