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

[John Snow <jsnow@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 4109 bytes --]