1. Patchew
  2. QEMU
  3. [PATCH 0/4] Python: Add 3.13 support, play linter whackamole
  4. View patch

[PATCH 3/4] iotests: Change imports for Python 3.13

John Snow posted 4 patches 5 months ago
Maintainers: John Snow <jsnow@redhat.com>, Cleber Rosa <crosa@redhat.com>, Beraldo Leal <bleal@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Thomas Huth <thuth@redhat.com>, Wainer dos Santos Moschetta <wainersm@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
[PATCH 3/4] iotests: Change imports for Python 3.13
Posted by John Snow 5 months ago 
Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to
make it the default system interpreter for Fedora 41.

They moved our cheese for where ContextManager lives; add a conditional
to locate it while we support both pre-3.9 and 3.13+.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/testenv.py    | 7 ++++++-
 tests/qemu-iotests/testrunner.py | 9 ++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 588f30a4f14..96d69e56963 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -25,7 +25,12 @@
 import random
 import subprocess
 import glob
-from typing import List, Dict, Any, Optional, ContextManager
+from typing import List, Dict, Any, Optional
+
+if sys.version_info >= (3, 9):
+    from contextlib import AbstractContextManager as ContextManager
+else:
+    from typing import ContextManager
 
 DEF_GDB_OPTIONS = 'localhost:12345'
 
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 7b322272e92..2e236c8fa39 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -27,11 +27,14 @@
 import shutil
 import sys
 from multiprocessing import Pool
-from typing import List, Optional, Any, Sequence, Dict, \
-        ContextManager
-
+from typing import List, Optional, Any, Sequence, Dict
 from testenv import TestEnv
 
+if sys.version_info >= (3, 9):
+    from contextlib import AbstractContextManager as ContextManager
+else:
+    from typing import ContextManager
+
 
 def silent_unlink(path: Path) -> None:
     try:
-- 
2.45.0
Re: [PATCH 3/4] iotests: Change imports for Python 3.13
Posted by Nir Soffer 4 months, 3 weeks ago 
On Thu, Jun 27, 2024 at 2:23 AM John Snow <jsnow@redhat.com> wrote:
>
> Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to
> make it the default system interpreter for Fedora 41.
>
> They moved our cheese for where ContextManager lives; add a conditional
> to locate it while we support both pre-3.9 and 3.13+.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/testenv.py    | 7 ++++++-
>  tests/qemu-iotests/testrunner.py | 9 ++++++---
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> index 588f30a4f14..96d69e56963 100644
> --- a/tests/qemu-iotests/testenv.py
> +++ b/tests/qemu-iotests/testenv.py
> @@ -25,7 +25,12 @@
>  import random
>  import subprocess
>  import glob
> -from typing import List, Dict, Any, Optional, ContextManager
> +from typing import List, Dict, Any, Optional
> +
> +if sys.version_info >= (3, 9):
> +    from contextlib import AbstractContextManager as ContextManager
> +else:
> +    from typing import ContextManager

It can be cleaner to add a compat module hiding the details so the
entire project
can have a single instance of this. Other code will just use:

    from compat import ContextManager

>
>  DEF_GDB_OPTIONS = 'localhost:12345'
>
> diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
> index 7b322272e92..2e236c8fa39 100644
> --- a/tests/qemu-iotests/testrunner.py
> +++ b/tests/qemu-iotests/testrunner.py
> @@ -27,11 +27,14 @@
>  import shutil
>  import sys
>  from multiprocessing import Pool
> -from typing import List, Optional, Any, Sequence, Dict, \
> -        ContextManager
> -
> +from typing import List, Optional, Any, Sequence, Dict
>  from testenv import TestEnv
>
> +if sys.version_info >= (3, 9):
> +    from contextlib import AbstractContextManager as ContextManager
> +else:
> +    from typing import ContextManager
> +
>
>  def silent_unlink(path: Path) -> None:
>      try:
> --
> 2.45.0
>
>
Re: [PATCH 3/4] iotests: Change imports for Python 3.13
Posted by John Snow 4 months, 3 weeks ago 
On Tue, Jul 2, 2024 at 7:52 AM Nir Soffer <nsoffer@redhat.com> wrote:

> On Thu, Jun 27, 2024 at 2:23 AM John Snow <jsnow@redhat.com> wrote:
> >
> > Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to
> > make it the default system interpreter for Fedora 41.
> >
> > They moved our cheese for where ContextManager lives; add a conditional
> > to locate it while we support both pre-3.9 and 3.13+.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  tests/qemu-iotests/testenv.py    | 7 ++++++-
> >  tests/qemu-iotests/testrunner.py | 9 ++++++---
> >  2 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/testenv.py
> b/tests/qemu-iotests/testenv.py
> > index 588f30a4f14..96d69e56963 100644
> > --- a/tests/qemu-iotests/testenv.py
> > +++ b/tests/qemu-iotests/testenv.py
> > @@ -25,7 +25,12 @@
> >  import random
> >  import subprocess
> >  import glob
> > -from typing import List, Dict, Any, Optional, ContextManager
> > +from typing import List, Dict, Any, Optional
> > +
> > +if sys.version_info >= (3, 9):
> > +    from contextlib import AbstractContextManager as ContextManager
> > +else:
> > +    from typing import ContextManager
>
> It can be cleaner to add a compat module hiding the details so the
> entire project
> can have a single instance of this. Other code will just use:
>
>     from compat import ContextManager
>

If there were more than two uses, I'd consider it. As it stands, a
compat.py module with just one import conditional in it doesn't seem worth
the hassle. Are there more cases of compatibility goop inside iotests that
need to be factored out to make it worth it?

--js
Re: [PATCH 3/4] iotests: Change imports for Python 3.13
Posted by Nir Soffer 4 months, 3 weeks ago 
> On 2 Jul 2024, at 17:44, John Snow <jsnow@redhat.com> wrote:
> 
> 
> 
> On Tue, Jul 2, 2024 at 7:52 AM Nir Soffer <nsoffer@redhat.com <mailto:nsoffer@redhat.com>> wrote:
>> On Thu, Jun 27, 2024 at 2:23 AM John Snow <jsnow@redhat.com <mailto:jsnow@redhat.com>> wrote:
>> >
>> > Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to
>> > make it the default system interpreter for Fedora 41.
>> >
>> > They moved our cheese for where ContextManager lives; add a conditional
>> > to locate it while we support both pre-3.9 and 3.13+.
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com <mailto:jsnow@redhat.com>>
>> > ---
>> >  tests/qemu-iotests/testenv.py    | 7 ++++++-
>> >  tests/qemu-iotests/testrunner.py | 9 ++++++---
>> >  2 files changed, 12 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
>> > index 588f30a4f14..96d69e56963 100644
>> > --- a/tests/qemu-iotests/testenv.py
>> > +++ b/tests/qemu-iotests/testenv.py
>> > @@ -25,7 +25,12 @@
>> >  import random
>> >  import subprocess
>> >  import glob
>> > -from typing import List, Dict, Any, Optional, ContextManager
>> > +from typing import List, Dict, Any, Optional
>> > +
>> > +if sys.version_info >= (3, 9):
>> > +    from contextlib import AbstractContextManager as ContextManager
>> > +else:
>> > +    from typing import ContextManager
>> 
>> It can be cleaner to add a compat module hiding the details so the
>> entire project
>> can have a single instance of this. Other code will just use:
>> 
>>     from compat import ContextManager
> 
> If there were more than two uses, I'd consider it. As it stands, a compat.py module with just one import conditional in it doesn't seem worth the hassle. Are there more cases of compatibility goop inside iotests that need to be factored out to make it worth it?

I don’t about other. For me even one instance is ugly enough :-)
Re: [PATCH 3/4] iotests: Change imports for Python 3.13
Posted by John Snow 4 months, 3 weeks ago 
On Tue, Jul 2, 2024, 1:51 PM Nir Soffer <nsoffer@redhat.com> wrote:

>
> On 2 Jul 2024, at 17:44, John Snow <jsnow@redhat.com> wrote:
>
>
>
> On Tue, Jul 2, 2024 at 7:52 AM Nir Soffer <nsoffer@redhat.com> wrote:
>
>> On Thu, Jun 27, 2024 at 2:23 AM John Snow <jsnow@redhat.com> wrote:
>> >
>> > Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to
>> > make it the default system interpreter for Fedora 41.
>> >
>> > They moved our cheese for where ContextManager lives; add a conditional
>> > to locate it while we support both pre-3.9 and 3.13+.
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>> > ---
>> >  tests/qemu-iotests/testenv.py    | 7 ++++++-
>> >  tests/qemu-iotests/testrunner.py | 9 ++++++---
>> >  2 files changed, 12 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/tests/qemu-iotests/testenv.py
>> b/tests/qemu-iotests/testenv.py
>> > index 588f30a4f14..96d69e56963 100644
>> > --- a/tests/qemu-iotests/testenv.py
>> > +++ b/tests/qemu-iotests/testenv.py
>> > @@ -25,7 +25,12 @@
>> >  import random
>> >  import subprocess
>> >  import glob
>> > -from typing import List, Dict, Any, Optional, ContextManager
>> > +from typing import List, Dict, Any, Optional
>> > +
>> > +if sys.version_info >= (3, 9):
>> > +    from contextlib import AbstractContextManager as ContextManager
>> > +else:
>> > +    from typing import ContextManager
>>
>> It can be cleaner to add a compat module hiding the details so the
>> entire project
>> can have a single instance of this. Other code will just use:
>>
>>     from compat import ContextManager
>>
>
> If there were more than two uses, I'd consider it. As it stands, a
> compat.py module with just one import conditional in it doesn't seem worth
> the hassle. Are there more cases of compatibility goop inside iotests that
> need to be factored out to make it worth it?
>
>
> I don’t about other. For me even one instance is ugly enough :-)
>

I was going to add it to qemu/utils, but then I remembered the
testenv/testrunner script here needs to operate without external
dependencies because part of the function of these modules is to *locate*
those dependencies.

Ehh. I'm going to say that repeating the import scaffolding in just two
places is fine enough for now and really not worth adding a compat.py for
*just* this. Let's just get the tests green.

--js
Re: [PATCH 3/4] iotests: Change imports for Python 3.13
Posted by John Snow 4 months, 3 weeks ago 
Ping - happy to merge this series myself but didn't wanna change iotests
without at least an ack from the lord of that castle.

On Wed, Jun 26, 2024, 7:22 PM John Snow <jsnow@redhat.com> wrote:

> Python 3.13 isn't out yet, but it's in beta and Fedora is ramping up to
> make it the default system interpreter for Fedora 41.
>
> They moved our cheese for where ContextManager lives; add a conditional
> to locate it while we support both pre-3.9 and 3.13+.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  tests/qemu-iotests/testenv.py    | 7 ++++++-
>  tests/qemu-iotests/testrunner.py | 9 ++++++---
>  2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
> index 588f30a4f14..96d69e56963 100644
> --- a/tests/qemu-iotests/testenv.py
> +++ b/tests/qemu-iotests/testenv.py
> @@ -25,7 +25,12 @@
>  import random
>  import subprocess
>  import glob
> -from typing import List, Dict, Any, Optional, ContextManager
> +from typing import List, Dict, Any, Optional
> +
> +if sys.version_info >= (3, 9):
> +    from contextlib import AbstractContextManager as ContextManager
> +else:
> +    from typing import ContextManager
>
>  DEF_GDB_OPTIONS = 'localhost:12345'
>
> diff --git a/tests/qemu-iotests/testrunner.py
> b/tests/qemu-iotests/testrunner.py
> index 7b322272e92..2e236c8fa39 100644
> --- a/tests/qemu-iotests/testrunner.py
> +++ b/tests/qemu-iotests/testrunner.py
> @@ -27,11 +27,14 @@
>  import shutil
>  import sys
>  from multiprocessing import Pool
> -from typing import List, Optional, Any, Sequence, Dict, \
> -        ContextManager
> -
> +from typing import List, Optional, Any, Sequence, Dict
>  from testenv import TestEnv
>
> +if sys.version_info >= (3, 9):
> +    from contextlib import AbstractContextManager as ContextManager
> +else:
> +    from typing import ContextManager
> +
>
>  def silent_unlink(path: Path) -> None:
>      try:
> --
> 2.45.0
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.