diff mbox series

[python] libvirtaio: Fix compat with python 3.7

Message ID 1628bcfd04594298769425871cc1d5b515aefea1.1529950820.git.crobinso@redhat.com
State New
Headers show
Series [python] libvirtaio: Fix compat with python 3.7 | expand

Commit Message

Cole Robinson June 25, 2018, 6:35 p.m. UTC
In python 3.7, async is now a keyword, so this throws a syntax error:

  File "/usr/lib64/python3.7/site-packages/libvirtaio.py", line 49
    from asyncio import async as ensure_future
                            ^
  SyntaxError: invalid syntax

Switch to getattr trickery to accomplish the same goal

Signed-off-by: Cole Robinson <crobinso@redhat.com>

---
 libvirtaio.py | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

If we don't care about python3 < 3.4.4 compat, we can just do

  from asyncio import ensure_future

-- 
2.17.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Comments

Cole Robinson June 26, 2018, 2:27 p.m. UTC | #1
On 06/25/2018 02:35 PM, Cole Robinson wrote:
> In python 3.7, async is now a keyword, so this throws a syntax error:

> 

>   File "/usr/lib64/python3.7/site-packages/libvirtaio.py", line 49

>     from asyncio import async as ensure_future

>                             ^

>   SyntaxError: invalid syntax

> 

> Switch to getattr trickery to accomplish the same goal

> 

> Signed-off-by: Cole Robinson <crobinso@redhat.com>

> ---

>  libvirtaio.py | 10 ++++++----

>  1 file changed, 6 insertions(+), 4 deletions(-)

> 

> If we don't care about python3 < 3.4.4 compat, we can just do

> 

>   from asyncio import ensure_future

> 


Also I meant to say, this was prompted by python3.7 rebase in Fedora:

https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/5DOPLN7K7GCI3T6ZMXDBIPJW5Y7ESGYS/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Andrea Bolognani June 26, 2018, 3:51 p.m. UTC | #2
On Mon, 2018-06-25 at 14:35 -0400, Cole Robinson wrote:
> If we don't care about python3 < 3.4.4 compat, we can just do

> 

>   from asyncio import ensure_future


Debian 8, which is still a supported target platform[1], only
has Python 3.4.2.


[1] At least for libvirt; I'm going to assume libvirt-python
    doesn't have its own support policy.
-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Pavel Hrdina June 27, 2018, 11:50 a.m. UTC | #3
On Mon, Jun 25, 2018 at 02:35:28PM -0400, Cole Robinson wrote:
> In python 3.7, async is now a keyword, so this throws a syntax error:

> 

>   File "/usr/lib64/python3.7/site-packages/libvirtaio.py", line 49

>     from asyncio import async as ensure_future

>                             ^

>   SyntaxError: invalid syntax

> 

> Switch to getattr trickery to accomplish the same goal

> 

> Signed-off-by: Cole Robinson <crobinso@redhat.com>

> ---

>  libvirtaio.py | 10 ++++++----

>  1 file changed, 6 insertions(+), 4 deletions(-)

> 

> If we don't care about python3 < 3.4.4 compat, we can just do

> 

>   from asyncio import ensure_future

> 

> diff --git a/libvirtaio.py b/libvirtaio.py

> index 1c432dd..9d517e6 100644

> --- a/libvirtaio.py

> +++ b/libvirtaio.py

> @@ -43,10 +43,12 @@ import warnings

>  

>  import libvirt

>  

> -try:

> -    from asyncio import ensure_future

> -except ImportError:

> -    from asyncio import async as ensure_future

> +# python < 3.4.4: we want 'async'

> +# python >= 3.4.4 < 3.7, we want 'ensure_future'


This is slightly misleading, I would remove the ' < 3.7' part as we
need 'ensure_future' even for that versions.  The following explanation
is good enough.

> +# python >= 3.7, 'async' is a reserved keyword, so we can't import it

> +ensure_future = getattr(asyncio, "ensure_future", None)

> +if not ensure_future:

> +    ensure_future = getattr(asyncio, "async")


Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Andrea Bolognani June 27, 2018, 11:50 a.m. UTC | #4
On Mon, 2018-06-25 at 14:35 -0400, Cole Robinson wrote:
[...]
> -try:

> -    from asyncio import ensure_future

> -except ImportError:

> -    from asyncio import async as ensure_future

> +# python < 3.4.4: we want 'async'

> +# python >= 3.4.4 < 3.7, we want 'ensure_future'

> +# python >= 3.7, 'async' is a reserved keyword, so we can't import it

> +ensure_future = getattr(asyncio, "ensure_future", None)

> +if not ensure_future:

> +    ensure_future = getattr(asyncio, "async")


Python is not exactly my forte, but the above makes sense to me
and it stood up to some testing across all platforms supported by
libvirt, so

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>


One nit is that the comment above the code doesn't IMHO describe
the situation properly: I would have worded it along the lines of

  # Python < 3.4.4 doesn't have 'ensure_future', so we have to fall
  # back to 'async'; however, since 'async' is a reserved keyword
  # in Python >= 3.7, we can't perform a straightforward import and
  # we have to resort to getattr() instead

I leave it up to you whether or not you want to reword the comment.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Cole Robinson June 27, 2018, 2:08 p.m. UTC | #5
On 06/27/2018 07:50 AM, Andrea Bolognani wrote:
> On Mon, 2018-06-25 at 14:35 -0400, Cole Robinson wrote:

> [...]

>> -try:

>> -    from asyncio import ensure_future

>> -except ImportError:

>> -    from asyncio import async as ensure_future

>> +# python < 3.4.4: we want 'async'

>> +# python >= 3.4.4 < 3.7, we want 'ensure_future'

>> +# python >= 3.7, 'async' is a reserved keyword, so we can't import it

>> +ensure_future = getattr(asyncio, "ensure_future", None)

>> +if not ensure_future:

>> +    ensure_future = getattr(asyncio, "async")

> 

> Python is not exactly my forte, but the above makes sense to me

> and it stood up to some testing across all platforms supported by

> libvirt, so

> 

>   Reviewed-by: Andrea Bolognani <abologna@redhat.com>

> 

> One nit is that the comment above the code doesn't IMHO describe

> the situation properly: I would have worded it along the lines of

> 

>   # Python < 3.4.4 doesn't have 'ensure_future', so we have to fall

>   # back to 'async'; however, since 'async' is a reserved keyword

>   # in Python >= 3.7, we can't perform a straightforward import and

>   # we have to resort to getattr() instead

> 

> I leave it up to you whether or not you want to reword the comment.

> 


I used your comment and pushed it now

Thanks,
Cole

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
diff mbox series

Patch

diff --git a/libvirtaio.py b/libvirtaio.py
index 1c432dd..9d517e6 100644
--- a/libvirtaio.py
+++ b/libvirtaio.py
@@ -43,10 +43,12 @@  import warnings
 
 import libvirt
 
-try:
-    from asyncio import ensure_future
-except ImportError:
-    from asyncio import async as ensure_future
+# python < 3.4.4: we want 'async'
+# python >= 3.4.4 < 3.7, we want 'ensure_future'
+# python >= 3.7, 'async' is a reserved keyword, so we can't import it
+ensure_future = getattr(asyncio, "ensure_future", None)
+if not ensure_future:
+    ensure_future = getattr(asyncio, "async")
 
 
 class Callback(object):