Re: libvirt-python: API change List → (Named)Tuple?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jul 27, 2020 at 08:32:29AM +0200, Philipp Hahn wrote:
> Hello,
> 
> Am 25.07.20 um 23:45 schrieb Philipp Hahn:
> > Am 27.04.20 um 15:44 schrieb Philipp Hahn:
> >> I'm working on adding PEP 484 type hints
> >> <https://www.python.org/dev/peps/pep-0484/> to the Python binding of
> >> libvirt.
> ...
> > I just opened a merge request
> > <https://gitlab.com/libvirt/libvirt-python/-/merge_requests/9> for my
> > code at <https://gitlab.com/pmhahn/libvirt-python/-/tree/typing>.
> 
> While working on that I stumbled over some annoyances in the current
> Python API: There are many methods which return a List[int], for example:
>   virDomainGetInfo
>   virDomainGetState
>   virDomainGetControlInfo
>   virDomainGetBlockInfo
>   virDomainGetJobInfo
>   virStoragePoolGetInfo
>   virStorageVolGetInfo
>   virStorageVolGetInfoFlags
> 
> There are more function returning similar information as plain List:
>   virNodeGetSecurityModel
>   virNodeGetSecurityModel
>   virDomainGetSecurityLabelList
>   virDomainBlockStats
>   virDomainInterfaceStats
>   virDomainGetVcpus
>   virNodeGetCPUMap
> 
> The worst offender is `virNodeGetInfo`, which returns `Tuple[str,
> int*7]`: The problem for type annotation is that `List` is unlimited in
> the number of elements, that is you cannot specify the number of entries
> the list must or will contain.
> Also all elements of a list must have the same (super-)type; for "str"
> and "int" of "virNodeGetInfo()" that would be "Any", which is not very
> useful here.
> 
> A better type for those `List`s would be `Tuple`, which has a fixed
> length and can have different types for each position.
> 
> But that would be an API change: In most cases users of those functions
> should not notice the difference as indexing tuples or lists is the same.
> It would break code where someone would do something like this:
>   ret = virFunction_returning_list()
>   ret += [1, 2, 3]

I would argue that ^this was never the intended way of using the
returned object and would lean towards using a named tuple, since like
you've pointed out it would be a transparent change in the vast majority
of cases.  However, since we don't document anywhere how the return
value should be treated, from that perspective it's still valid to
change the returned list for whatever purposes and therefore we are
breaking the API contract, which we can't do even though the change
itself is reasonable.

Regards,
Erik




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux