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

[Nir Soffer <nsoffer@redhat.com>]

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


> 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 :-)


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