-
Notifications
You must be signed in to change notification settings - Fork 242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
qemu_storage: adapt the collection mapping function #3916
Conversation
5182503
to
f57ea79
Compare
(1/1) Host_RHEL.m10.u0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.qemu_img_convert_with_backing_file.q35: STARTED |
@zhencliu Could you please help to review here? Thanks. BTW, some checks were not successful, but I checked the details seem that the errors are not related to my fix. |
virttest/qemu_storage.py
Outdated
@@ -11,6 +11,7 @@ | |||
import os | |||
import re | |||
import string | |||
from distutils import sysconfig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about using try except, e.g.
try:
from collections.abc import Mapping
except ImportError:
from collections import Mapping
Check BytesIO in utils_misc.py for an example
BTW, we cannot compare string for the version, e.g.
>>> '3.6' < '3.10'
False
But we can use tuple comparison, e.g.
sys.version_info < (3, 10)
>>> (3, 6) < (3, 10)
True
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, agree with you, your method makes more sense. I have updated the fix. Thanks.
013e18e
to
32a3043
Compare
(1/1) Host_RHEL.m10.u0.ovmf.qcow2.virtio_scsi.up.virtio_net.Guest.RHEL.9.5.0.x86_64.io-github-autotest-qemu.qemu_img_convert_with_backing_file.q35: PASS (264.22 s) @zhencliu Could you please help to take another review? Thanks. |
@YongxueHong Could you please help to review here? Thanks. |
try: | ||
from collections.abc import Mapping | ||
except ImportError: | ||
from collections import Mapping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like
avocado-vt/virttest/qemu_storage.py
Line 8 in 48b5b3d
import collections |
is useless, we could delete it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @hellohellenmao and @YongxueHong , actually, we use collections.OrderedDict when getting image opts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's. I noticed this just now. So I keep the original changes now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @hellohellenmao and @YongxueHong , actually, we use collections.OrderedDict when getting image opts
Thanks, I got it, I missed it.
2e4b977
to
884d5a2
Compare
@YongxueHong Please help to review again, thanks. |
Adapt the function according to different python versions Signed-off-by: Tingting Mao <timao@redhat.com>
Adapt the function according to different python versions
ID: 2413