[Python3] Python3 migrate fix key error when parsing conn_graph_facts by lerry-lee · Pull Request #7196 · sonic-net/sonic-mgmt

lerry-lee · 2023-01-06T09:50:43Z

Description of PR

When running pc/test_lag_2.py, and some cases under directory pfcwd/ in Python3 environmet, there are some key errors like the below:

KeyError('str2-7260cx3-acs-fan-15')
Traceback (most recent call last):
  File "/azp/_work/25/s/tests/common/plugins/log_section_start/__init__.py", line 84, in _fixture_generator_decorator
    res = next(it)
  File "/azp/_work/25/s/tests/pfcwd/test_pfcwd_timer_accuracy.py", line 75, in pfcwd_timer_setup_restore
    storm_handle = set_storm_params(dut, fanout_info, fanout, peer_params)
  File "/azp/_work/25/s/tests/pfcwd/test_pfcwd_timer_accuracy.py", line 133, in set_storm_params
    storm_handle = PFCStorm(dut, fanout_info, fanout, pfc_queue_idx=pfc_queue_index,
  File "/azp/_work/25/s/tests/common/helpers/pfc_storm.py", line 51, in __init__
    self.peer_device = self.fanout_hosts[self.peer_info['peerdevice']]
KeyError: 'str2-7260cx3-acs-fan-15'

Actually, it is caused by these codes (When parsing conn_graph_facts):

@pytest.fixture(scope="module")
def fanouthosts(ansible_adhoc, conn_graph_facts, creds, ditheists):      # noqa F811
    """
    Shortcut fixture for getting Fanout hosts
    """

    dev_conn = conn_graph_facts.get('device_conn', {})
    fanout_hosts = {}
    # WA for virtual testbed which has no fanout
    try:
        for dut_host, value in dev_conn.items():
            duthost = duthosts[dut_host]

In Python2 env, the dut_host(the key in dev_conn.items()) is unicode object, but in Python3 is AnsileUnsafeText object, so that adding convert to str explicitly can solve this issue.

In Python2, dict.keys()/dict.values() returns list object, but in Python3 returns an iterable but not indexable object. So that need to convert to list.

The Python "NameError name 'unicode' is not defined" occurs when using the unicode object in Python 3. To solve the error, replace all calls to unicode() with str() because unicode was renamed to str in Python 3.

Type of change

Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)

Back port request

201911
202012
202205

Approach

What is the motivation for this PR?

Python3 migrate fix key error.

How did you do it?

Add explicit convert.

How did you verify/test it?

Run TC and no key error.

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

1. In Python2, dict.keys() returns list object, but in Python3 returns an iterable but not indexable object. So that need to convert to list. 2. In Python2, the key in dev_conn.items() is unicode object, but in Python3 is AnsileUnsafeText. So that need to convert to str. Signed-off-by: Chun'ang Li <chunangli@microsoft.com>

1. The Python "NameError name 'unicode' is not defined" occurs when using the unicode object in Python 3. To solve the error, replace all calls to unicode() with str() because unicode was renamed to str in Python 3. 2. In Python2, dict.keys() returns list object, but in Python3 returns an iterable but not indexable object. So that need to convert to list. Signed-off-by: Chun'ang Li <chunangli@microsoft.com>

…g the unicode object in Python 3. To solve the error, replace all calls to unicode() with str() because unicode was renamed to str in Python 3. Signed-off-by: Chun'ang Li <chunangli@microsoft.com>

…n iterable but not indexable object. So that need to convert to list. Signed-off-by: Chun'ang Li <chunangli@microsoft.com>

…ey-error

Signed-off-by: Chun'ang Li <chunangli@microsoft.com>

wangxin · 2023-01-10T05:56:35Z

tests/conftest.py

-            duthost = duthosts[dut_host]
+            # In Python2, the key in dev_conn.items() is unicode object, but in Python3 is AnsileUnsafeText.
+            # So that convert to str explicitly.
+            duthost = duthosts[str(dut_host)]


Other test scripts using this fixture also may run into such issue. Suggest to fix this issue from ansible/library/conn_graph_facts.py.

Thanks for the good suggestion! But I tried to modify the ansible/library/conn_graph_facts.py like rename the key device_conn, but it didn't work:

results = { 'device_info': lab_graph.devices, 'device_conn': lab_graph.links, 'device_port_vlans': lab_graph.vlanport, }

Currently, I found conn_graph_facts got from here in tests/common/fixtures/conn_graph_facts.py:

conn_graph_facts = localhost.conn_graph_facts( **kargs)["ansible_facts"]

So, I added a method to make some converts after this conn_graph_fact was got if using Python3. This can also fix this issue.

conn_graph_facts = localhost.conn_graph_facts( **kargs)["ansible_facts"] return key_convert2str(conn_graph_facts)

def key_convert2str(conn_graph_facts): """ In Python2, some key type are unicode, but In Python3, are AnsibleUnsafeText. Convert them to str. Currently, convert conn_graph_facts['device_conn']. """ # If Python2, do not change if sys.version_info[0] < 3: return conn_graph_facts # Else, convert result = copy.deepcopy(conn_graph_facts) for key, value in conn_graph_facts['device_conn'].items(): result['device_conn'][str(key)] = result['device_conn'].pop(str(key)) return result

Signed-off-by: Chun'ang Li <chunangli@microsoft.com>

wangxin · 2023-01-16T08:14:11Z

tests/common/fixtures/conn_graph_facts.py

+
+
+import sys
+import copy


It would be better to import at the top of file.

wangxin · 2023-01-16T08:21:11Z

tests/common/fixtures/conn_graph_facts.py

                conn_graph_facts = localhost.conn_graph_facts(
                    **kargs)["ansible_facts"]
-                return conn_graph_facts
+                return key_convert2str(conn_graph_facts)


Fixing from pytest fixture side only resolves the issue we encountered in pytest.
The conn_graph_facts ansible module may be called in ansible or other places. It would be better to resolve the encoding issue directly from the conn_graph_facts ansible module located at sonic-mgmt/ansible/library/conn_graph_facts.py

lerry-lee · 2023-02-06T09:12:33Z

New PR #7399 replace this.

lerry-lee added 6 commits January 5, 2023 16:31

The Python "NameError name 'unicode' is not defined" occurs when usin…

766048d

…g the unicode object in Python 3. To solve the error, replace all calls to unicode() with str() because unicode was renamed to str in Python 3. Signed-off-by: Chun'ang Li <chunangli@microsoft.com>

In Python2, dict.keys() returns list object, but in Python3 returns a…

17cf0a9

…n iterable but not indexable object. So that need to convert to list. Signed-off-by: Chun'ang Li <chunangli@microsoft.com>

Merge remote-tracking branch 'upstream/master' into py3-migrate-fix-k…

d3fc855

…ey-error

Add some comments

29141eb

Signed-off-by: Chun'ang Li <chunangli@microsoft.com>

lerry-lee requested review from wangxin and wsycqyz January 6, 2023 09:51

lerry-lee changed the title ~~[Python3] Python3 migrate fix key error~~ [Python3] Python3 migrate fix key error when getting fanouthosts Jan 6, 2023

wsycqyz previously approved these changes Jan 6, 2023

View reviewed changes

Reformat code

06d46f5

Signed-off-by: Chun'ang Li <chunangli@microsoft.com>

lerry-lee dismissed wsycqyz’s stale review via 06d46f5 January 6, 2023 10:12

wangxin reviewed Jan 10, 2023

View reviewed changes

lerry-lee changed the title ~~[Python3] Python3 migrate fix key error when getting fanouthosts~~ [Python3] Python3 migrate fix key error when parsing conn_graph_facts Jan 13, 2023

lerry-lee added 2 commits January 13, 2023 17:28

After get conn_graph_facts, do convert some key type to str if Python3.

ce9c505

Signed-off-by: Chun'ang Li <chunangli@microsoft.com>

Improve comments.

2d8b328

Signed-off-by: Chun'ang Li <chunangli@microsoft.com>

wangxin reviewed Jan 16, 2023

View reviewed changes

fix

cf5c685

lerry-lee closed this Feb 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Python3] Python3 migrate fix key error when parsing conn_graph_facts#7196

[Python3] Python3 migrate fix key error when parsing conn_graph_facts#7196
lerry-lee wants to merge 10 commits intosonic-net:masterfrom
lerry-lee:py3-migrate-fix-key-error

lerry-lee commented Jan 6, 2023 •

edited

Loading

Uh oh!

wangxin Jan 10, 2023

Uh oh!

lerry-lee Jan 13, 2023 •

edited

Loading

Uh oh!

wangxin Jan 16, 2023

Uh oh!

wangxin Jan 16, 2023

Uh oh!

lerry-lee commented Feb 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

lerry-lee commented Jan 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

wangxin Jan 10, 2023

Choose a reason for hiding this comment

Uh oh!

lerry-lee Jan 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wangxin Jan 16, 2023

Choose a reason for hiding this comment

Uh oh!

wangxin Jan 16, 2023

Choose a reason for hiding this comment

Uh oh!

lerry-lee commented Feb 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lerry-lee commented Jan 6, 2023 •

edited

Loading

lerry-lee Jan 13, 2023 •

edited

Loading