[PATCH v7 25/33] iotests.py: VM: add own __enter__ method

Vladimir Sementsov-Ogievskiy posted 33 patches 4 years, 6 months ago
There is a newer version of this series
[PATCH v7 25/33] iotests.py: VM: add own __enter__ method
Posted by Vladimir Sementsov-Ogievskiy 4 years, 6 months ago
In superclass __enter__ method is defined with return value type hint
'QEMUMachine'. So, mypy thinks that return value of VM.__enter__ is
QEMUMachine. Let's redefine __enter__ in VM class, to give it correct
type hint.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 89663dac06..025e288ddd 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -568,6 +568,10 @@ def remote_filename(path):
 class VM(qtest.QEMUQtestMachine):
     '''A QEMU VM'''
 
+    # Redefine __enter__ with proper type hint
+    def __enter__(self) -> 'VM':
+        return self
+
     def __init__(self, path_suffix=''):
         name = "qemu%s-%d" % (path_suffix, os.getpid())
         super().__init__(qemu_prog, qemu_opts, name=name,
-- 
2.29.2


Re: [PATCH v7 25/33] iotests.py: VM: add own __enter__ method
Posted by John Snow 4 years, 6 months ago
On Wed, Aug 4, 2021 at 5:39 AM Vladimir Sementsov-Ogievskiy <
vsementsov@virtuozzo.com> wrote:

> In superclass __enter__ method is defined with return value type hint
> 'QEMUMachine'. So, mypy thinks that return value of VM.__enter__ is
> QEMUMachine. Let's redefine __enter__ in VM class, to give it correct
> type hint.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/iotests.py | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 89663dac06..025e288ddd 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -568,6 +568,10 @@ def remote_filename(path):
>  class VM(qtest.QEMUQtestMachine):
>      '''A QEMU VM'''
>
> +    # Redefine __enter__ with proper type hint
> +    def __enter__(self) -> 'VM':
> +        return self
> +
>      def __init__(self, path_suffix=''):
>          name = "qemu%s-%d" % (path_suffix, os.getpid())
>          super().__init__(qemu_prog, qemu_opts, name=name,
> --
> 2.29.2
>

First and foremost:

Reviewed-by: John Snow <jsnow@redhat.com>
Acked-by: John Snow <jsnow@redhat.com>

A more durable approach might be to annotate QEMUMachine differently such
that subclasses get the right types automatically. See if this following
snippet works instead of adding a new __enter__ method.

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 971ed7e8c6a..2e410103569 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -36,6 +36,7 @@
     Sequence,
     Tuple,
     Type,
+    TypeVar,
 )

 from qemu.qmp import (  # pylint: disable=import-error
@@ -73,6 +74,9 @@ class AbnormalShutdown(QEMUMachineError):
     """


+_T = TypeVar('_T', bound='QEMUMachine')
+
+
 class QEMUMachine:
     """
     A QEMU VM.
@@ -166,7 +170,7 @@ def __init__(self,
         self._remove_files: List[str] = []
         self._user_killed = False

-    def __enter__(self) -> 'QEMUMachine':
+    def __enter__(self: _T) -> _T:
         return self

     def __exit__(self,
@@ -182,8 +186,8 @@ def add_monitor_null(self) -> None:
         self._args.append('-monitor')
         self._args.append('null')

-    def add_fd(self, fd: int, fdset: int,
-               opaque: str, opts: str = '') -> 'QEMUMachine':
+    def add_fd(self: _T, fd: int, fdset: int,
+               opaque: str, opts: str = '') -> _T:
         """
         Pass a file descriptor to the VM
         """
Re: [PATCH v7 25/33] iotests.py: VM: add own __enter__ method
Posted by Vladimir Sementsov-Ogievskiy 4 years, 5 months ago
04.08.2021 19:27, John Snow wrote:
> 
> On Wed, Aug 4, 2021 at 5:39 AM Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com <mailto:vsementsov@virtuozzo.com>> wrote:
> 
>     In superclass __enter__ method is defined with return value type hint
>     'QEMUMachine'. So, mypy thinks that return value of VM.__enter__ is
>     QEMUMachine. Let's redefine __enter__ in VM class, to give it correct
>     type hint.
> 
>     Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com <mailto:vsementsov@virtuozzo.com>>
>     Reviewed-by: Max Reitz <mreitz@redhat.com <mailto:mreitz@redhat.com>>
>     ---
>       tests/qemu-iotests/iotests.py | 4 ++++
>       1 file changed, 4 insertions(+)
> 
>     diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>     index 89663dac06..025e288ddd 100644
>     --- a/tests/qemu-iotests/iotests.py
>     +++ b/tests/qemu-iotests/iotests.py
>     @@ -568,6 +568,10 @@ def remote_filename(path):
>       class VM(qtest.QEMUQtestMachine):
>           '''A QEMU VM'''
> 
>     +    # Redefine __enter__ with proper type hint
>     +    def __enter__(self) -> 'VM':
>     +        return self
>     +
>           def __init__(self, path_suffix=''):
>               name = "qemu%s-%d" % (path_suffix, os.getpid())
>               super().__init__(qemu_prog, qemu_opts, name=name,
>     -- 
>     2.29.2
> 
> 
> First and foremost:
> 
> Reviewed-by: John Snow <jsnow@redhat.com <mailto:jsnow@redhat.com>>
> Acked-by: John Snow <jsnow@redhat.com <mailto:jsnow@redhat.com>>
> 
> A more durable approach might be to annotate QEMUMachine differently such that subclasses get the right types automatically. See if this following snippet works instead of adding a new __enter__ method.
> 
> diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
> index 971ed7e8c6a..2e410103569 100644
> --- a/python/qemu/machine/machine.py
> +++ b/python/qemu/machine/machine.py
> @@ -36,6 +36,7 @@
>       Sequence,
>       Tuple,
>       Type,
> +    TypeVar,
>   )
> 
>   from qemu.qmp import (  # pylint: disable=import-error
> @@ -73,6 +74,9 @@ class AbnormalShutdown(QEMUMachineError):
>       """
> 
> 
> +_T = TypeVar('_T', bound='QEMUMachine')
> +
> +
>   class QEMUMachine:
>       """
>       A QEMU VM.
> @@ -166,7 +170,7 @@ def __init__(self,
>           self._remove_files: List[str] = []
>           self._user_killed = False
> 
> -    def __enter__(self) -> 'QEMUMachine':
> +    def __enter__(self: _T) -> _T:
>           return self
> 
>       def __exit__(self,
> @@ -182,8 +186,8 @@ def add_monitor_null(self) -> None:
>           self._args.append('-monitor')
>           self._args.append('null')
> 
> -    def add_fd(self, fd: int, fdset: int,
> -               opaque: str, opts: str = '') -> 'QEMUMachine':
> +    def add_fd(self: _T, fd: int, fdset: int,
> +               opaque: str, opts: str = '') -> _T:
>           """
>           Pass a file descriptor to the VM
>           """

That looks better, I'll go this way, thanks!

-- 
Best regards,
Vladimir