Skip to content

Commit

Permalink
Fix Python3 RuntimeError - dictionary changed size during iteration (#…
Browse files Browse the repository at this point in the history
…6646)

Summary:
Fix Python3 RuntimeError - dictionary changed size during iteration

What is the motivation for this PR?
Python3 migration: In Python3, you cannot add/remove elements in a dictionary while iterating through it.

How did you do it?
1. Use copy(). This way you iterate over the original dictionary fields and on the fly can change the desired dict.
2. Fix pre-commit issues.

How did you verify/test it?
Both Python2 and Python3 can run bgp/test_bgp_facts.py successfully.
  • Loading branch information
wsycqyz committed Oct 28, 2022
1 parent dbb5e8d commit 979eb20
Showing 1 changed file with 17 additions and 8 deletions.
25 changes: 17 additions & 8 deletions tests/common/plugins/conditional_mark/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def read_asic_name(hwsku):
with open(asic_name_file) as f:
asic_name = yaml.safe_load(f)

for key, value in asic_name.items():
for key, value in asic_name.copy().items():
if ('td' not in key) and ('th' not in key) and ('spc' not in key):
asic_name.pop(key)

Expand All @@ -123,9 +123,10 @@ def read_asic_name(hwsku):

return "unknown"

except IOError as e:
except IOError:
return None


def load_dut_basic_facts(inv_name, dut_name):
"""Run 'ansible -m dut_basic_facts' command to get some basic DUT facts.
Expand Down Expand Up @@ -155,6 +156,7 @@ def load_dut_basic_facts(inv_name, dut_name):

return results


def get_basic_facts(session):
testbed_name = session.config.option.testbed

Expand Down Expand Up @@ -209,6 +211,7 @@ def load_minigraph_facts(inv_name, dut_name):

return results


def load_config_facts(inv_name, dut_name):
"""Run 'ansible -m config_facts -a 'host={{hostname}} source='persistent' ' command to get some basic config facts.
Expand All @@ -225,7 +228,8 @@ def load_config_facts(inv_name, dut_name):
logger.info('Getting config basic facts')
try:
# get config basic faces
ansible_cmd = ['ansible', '-m', 'config_facts', '-i', '../ansible/{}'.format(inv_name), '{}'.format(dut_name), '-a', 'host={} source=\'persistent\''.format(dut_name)]
ansible_cmd = ['ansible', '-m', 'config_facts', '-i', '../ansible/{}'.format(inv_name),
'{}'.format(dut_name), '-a', 'host={} source=\'persistent\''.format(dut_name)]
raw_output = subprocess.check_output(ansible_cmd).decode('utf-8')
logger.debug('raw config basic facts:\n{}'.format(raw_output))
output_fields = raw_output.split('SUCCESS =>', 1)
Expand All @@ -239,6 +243,7 @@ def load_config_facts(inv_name, dut_name):

return results


def load_switch_capabilities_facts(inv_name, dut_name):
"""Run 'ansible -m switch_capabilities_facts' command to get some basic config facts.
Expand Down Expand Up @@ -267,6 +272,7 @@ def load_switch_capabilities_facts(inv_name, dut_name):

return results


def load_console_facts(inv_name, dut_name):
"""Run 'ansible -m console_facts' command to get some basic console facts.
Expand Down Expand Up @@ -295,6 +301,7 @@ def load_console_facts(inv_name, dut_name):

return results


def load_basic_facts(session):
"""Load some basic facts that can be used in condition statement evaluation.
Expand Down Expand Up @@ -446,15 +453,15 @@ def evaluate_condition(dynamic_update_skip_reason, mark_details, condition, basi
if condition_result and dynamic_update_skip_reason:
mark_details['reason'].append(condition)
return condition_result
except Exception as e:
except Exception:
logger.error('Failed to evaluate condition, raw_condition={}, condition_str={}'.format(
condition,
condition_str))
return False


def evaluate_conditions(dynamic_update_skip_reason, mark_details, conditions, basic_facts, \
conditions_logical_operator, session):
def evaluate_conditions(dynamic_update_skip_reason, mark_details, conditions, basic_facts,
conditions_logical_operator, session):
"""Evaluate all the condition strings.
Evaluate a single condition or multiple conditions. If multiple conditions are supplied, apply AND or OR
Expand All @@ -478,9 +485,11 @@ def evaluate_conditions(dynamic_update_skip_reason, mark_details, conditions, ba
if isinstance(conditions, list):
# Apply 'AND' or 'OR' operation to list of conditions based on conditions_logical_operator(by default 'AND')
if conditions_logical_operator == 'OR':
return any([evaluate_condition(dynamic_update_skip_reason, mark_details, c, basic_facts, session) for c in conditions])
return any([evaluate_condition(dynamic_update_skip_reason, mark_details, c, basic_facts, session)
for c in conditions])
else:
return all([evaluate_condition(dynamic_update_skip_reason, mark_details, c, basic_facts, session) for c in conditions])
return all([evaluate_condition(dynamic_update_skip_reason, mark_details, c, basic_facts, session)
for c in conditions])
else:
if conditions is None or conditions.strip() == '':
return True
Expand Down

0 comments on commit 979eb20

Please sign in to comment.