qemu-arm
[Top][All Lists]
Advanced

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

Re: [PULL 1/9] Python/iotests: Add type hint for nbd module


From: Eric Blake
Subject: Re: [PULL 1/9] Python/iotests: Add type hint for nbd module
Date: Thu, 5 Oct 2023 09:05:47 -0500
User-agent: NeoMutt/20230517

On Wed, Oct 04, 2023 at 03:46:05PM -0400, John Snow wrote:
> The test bails gracefully if this module isn't installed, but linters
> need a little help understanding that. It's enough to just declare the
> type in this case.
> 
> (Fixes pylint complaining about use of an uninitialized variable because
> it isn't wise enough to understand the notrun call is noreturn.)
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/tests/nbd-multiconn | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Since there's questions about this pull request seeming to be the
first time this patch has appeared on list, I'll go ahead and review
it here on the assumption that a v2 pull request is warranted.

> 
> diff --git a/tests/qemu-iotests/tests/nbd-multiconn 
> b/tests/qemu-iotests/tests/nbd-multiconn
> index 478a1eaba27..7e686a786ea 100755
> --- a/tests/qemu-iotests/tests/nbd-multiconn
> +++ b/tests/qemu-iotests/tests/nbd-multiconn
> @@ -20,6 +20,8 @@
>  
>  import os
>  from contextlib import contextmanager
> +from types import ModuleType
> +
>  import iotests
>  from iotests import qemu_img_create, qemu_io
>  
> @@ -28,7 +30,7 @@ disk = os.path.join(iotests.test_dir, 'disk')
>  size = '4M'
>  nbd_sock = os.path.join(iotests.sock_dir, 'nbd_sock')
>  nbd_uri = 'nbd+unix:///{}?socket=' + nbd_sock
> -
> +nbd: ModuleType

Is it possible to put this closer to the code actually using 'nbd', as
in this region?

| if __name__ == '__main__':
|     try:
|         # Easier to use libnbd than to try and set up parallel
|         # 'qemu-nbd --list' or 'qemu-io' processes, but not all systems
|         # have libnbd installed.
|         import nbd  # type: ignore

but then again, open_nbd() right after your current location utilizes
the variable, so I guess not.  I trust your judgment on silencing the
linters, so whether or not you move it (if moving is even possible),

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org




reply via email to

[Prev in Thread] Current Thread [Next in Thread]