[fast-reboot] Fix unicode issue in ipaddress for python2 #1164
Conversation
The 'ipaddress.ip_interface' accepts only unicode str as input, which is default in python3. However, an exception will raise when running in python2. This commit fix the issue. Signed-off-by: bingwang <[email protected]>
|
Retest this please |
| @@ -39,7 +40,7 @@ def generate_neighbor_entries(filename, all_available_macs): | |||
| arp_output.append(obj) | |||
|
|
|||
| ip_addr = key.split(':')[2] | |||
| if ipaddress.ip_interface(ip_addr).ip.version != 4: | |||
| if ipaddress.ip_interface(str(ip_addr)).ip.version != 4: | |||
There was a problem hiding this comment.
Would this fix issue:5580? One comment is that: can you make this Python3 friendly by checking the Python version?
There was a problem hiding this comment.
FYI, I have a draft PR where I am working on adding support for Python 3. You can look at this PR to see if it will play nice: #1128
There was a problem hiding this comment.
Would this fix issue:5580? One comment is that: can you make this Python3 friendly by checking the Python version?
Yes. The update is friendly to both python 2 and python 3. I have verified on both python version.
admin@str-dx010-acs-4:~$ python3
Python 3.7.3 (default, Jul 25 2020, 13:03:44)
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import ipaddress
>>> from builtins import str
>>> ip_addr = '192.168.0.1'
>>> ipaddress.ip_interface(str(ip_addr)).ip.version
4
>>> ipv6_addr = 'fc00:2::32'
>>> ipaddress.ip_interface(str(ipv6_addr)).ip.version
6
>>> exit()
admin@str-dx010-acs-4:~$ python
Python 2.7.16 (default, Oct 10 2019, 22:02:15)
[GCC 8.3.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import ipaddress
>>> from builtins import str
>>> ip_addr = '192.168.0.1'
>>> ipaddress.ip_interface(str(ip_addr)).ip.version
4
>>> ipv6_addr = 'fc00:2::32'
>>> ipaddress.ip_interface(str(ipv6_addr)).ip.version
6
There was a problem hiding this comment.
Thanks @bingwang-ms Can you please have a look at ipv6 splitting function below? it does not look that this would work with longer prefixes. Could be in a separate PR.
There was a problem hiding this comment.
Sure. Do you mean this line?
https://github.com/Azure/sonic-utilities/blob/011117bebcf05f6ef17a3d63155dec630ae88cc3/scripts/fast-reboot-dump.py#L45
What's the problem?
There was a problem hiding this comment.
nevermind. it is splitting the key and recover the Ipv6 from the key and not the ipaddr. it was an oversight.
There was a problem hiding this comment.
Would this fix issue:5580? One comment is that: can you make this Python3 friendly by checking the Python version?
Sorry for my last unclear reply. The issue sonic-net/sonic-buildimage#5580 is not fixed by this PR. The unicode issue is not found in fast-reboot-dump.py in 201911 branch.
|
This is similar to PR:1148 |
|
this is needed for 201911 as well. Not sure if needed for 20206. |
|
Does this PR make #1148 obsolete? |
Yes. Indeed. Sorry for not searching enough. But I think the fix in this PR is more elegant. |
- What I did
The
ipaddress.ip_interfaceaccepts only unicode string as input. However, an exception will be raised on python 2.As a result, fast-reboot aborted with error code 2.
This PR is to fix the issue.
- How I did it
Use
strinbuiltinsto handle the conversion of unicode.- How to verify it
Verified on both python2 and python3.
For python2, running
fast-reboot, and the command complete successfully without exception.For python3, a demo script is tested to verify the update.
- Previous command output (if the output of a command-line utility has changed)
N/A
- New command output (if the output of a command-line utility has changed)
N/A