Add a tool to convert port alias when used as bash pipe #135
Conversation
scripts/port2alias
Outdated
| start = 0 | ||
| end = 0 | ||
| while end < len(line): | ||
| if line[end].isalnum(): |
There was a problem hiding this comment.
here, you have an assumption that the port name does not contain char other than alnum. For example, the port name cannot be et5_1 #Resolved
There was a problem hiding this comment.
What's the legal charset for sonic interface names? I thought only alnum characters are allowed. #Closed
scripts/port2alias
Outdated
| platform = tokens[1] | ||
| return (platform, hwsku) | ||
|
|
||
| def main(): |
There was a problem hiding this comment.
it is probably good to have a unit test for this string conversion. We need to have some positive and negative case.
for example,
Ethernet3 should not be converted if Ethernet3 is not in the alias map.
Ethernet43 should not be converted if Etherent4 is in but not Ethernet43,
Ethernet4: should be converted if Ethernet4 is in. #Resolved
scripts/port2alias
Outdated
| else: | ||
| # End of a word | ||
| word = line[start:end] | ||
| if port.has_key(word): |
There was a problem hiding this comment.
Consider if word in port: (or if word in port.keys(): to make it more understandable at first glance) instead. More portable for the future as it appears has_key() is deprecated and has been removed in Python 3. #Resolved
|
retest this please |
3 similar comments
|
retest this please |
|
retest this please |
|
retest this please |
|
retest this please |
2 similar comments
|
retest this please |
|
retest this please |
Why I did this? xcvrd unit test failed when building it with python3: ``` 17:23:50 _____________________ ERROR collecting tests/test_xcvrd.py _____________________ 17:23:50 tests/test_xcvrd.py:36: in <module> 17:23:50 class TestXcvrdScript(object): 17:23:50 tests/test_xcvrd.py:41: in TestXcvrdScript 17:23:50 @patch('xcvrd.xcvrd.logical_port_name_to_physical_port_list', MagicMock(return_value=[0])) 17:23:50 E NameError: name 'patch' is not defined ``` How I did this? import the package patch
Signed-off-by: vaibhav-dahiya <[email protected]>
Sample Usage (on a S6000):
Depends on sonic-net/sonic-buildimage#1072