| Message ID | 20240626232230.408004-4-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Python: Add 3.13 support, play linter whackamole | expand |
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 > >
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 > >
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
> 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 :-)
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
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:
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(-)