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