Re: [PULL 3/5] qemu-iotests: 300: Add test case for modifying persistence of bitmap

[John Snow <jsnow@redhat.com>]

On 2/15/21 3:26 PM, Eric Blake wrote:
> On 2/15/21 1:00 PM, John Snow wrote:
>> On 2/15/21 1:25 PM, Eric Blake wrote:
>>> -BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]]
>>> +BlockBitmapMapping = List[Dict[str,
>>> +                               Union[str,
>>> +                                     List[Dict[str,
>>> +                                               Union[str, Dict[str,
>>> bool]]]]]]]
>>
>> That looks *very* beefy.
>>
>> Is the Union because that union is valid for every key, or because every
>> key has a potentially different value that is specific to that key?
>>
>> if it's the latter, I'd ditch the Union and just go with:
>>
>> Dict[str, object], or
>> Dict[str, Any]
>>
>> object: will allow any type, but keeps strict checking enabled. If you
>> try to use that value later on without a cast, mypy will warn you if you
>> are using it in a manner not guaranteed by the "object" type. Can be
>> useful if you are passing values to a function that already does RTTI to
>> determine behavior.
> 
> We're in luck; both 297 and 300 still pass with this applied on top of
> my previous attempt:
> 
> diff --git i/tests/qemu-iotests/300 w/tests/qemu-iotests/300
> index 7501bd1018e2..adb927629747 100755
> --- i/tests/qemu-iotests/300
> +++ w/tests/qemu-iotests/300
> @@ -22,7 +22,7 @@
>   import os
>   import random
>   import re
> -from typing import Dict, List, Optional, Union
> +from typing import Dict, List, Optional
> 
>   import iotests
> 
> @@ -30,10 +30,7 @@ import iotests
>   # pylint: disable=wrong-import-order
>   import qemu
> 
> -BlockBitmapMapping = List[Dict[str,
> -                               Union[str,
> -                                     List[Dict[str,
> -                                               Union[str, Dict[str,
> bool]]]]]]]
> +BlockBitmapMapping = List[Dict[str, object]]
> 

nice :)

>   mig_sock = os.path.join(iotests.sock_dir, 'mig_sock')
> 
> 
> 
>>
>> Any: Also allows any type, but enables gradual typing. If you later
>> "assume" the type of this value, mypy will say nothing. Can be useful
>> when you've just got a job to do and the right tool would have been a
>> recursive type or a TypedDict (unavailable in Python 3.6.)
> 
> I'm not too worried about needing to further enhance the type-checking
> on an individual iotest.
> 

Yes, Agreed. I have been going very "overboard" with the python and QAPI 
types, but I consider those libraries that might have need of such 
pedantic types.

The iotests themselves? eh.

Just figured I'd give you a range of options to choose from and you'd 
pick the best one.

--js


PS: I really really really really really wish that 3.6 had TypedDict.