Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[cfggen] Conform With Python 3 Syntax #5154

Merged
merged 6 commits into from
Aug 19, 2020
Merged

[cfggen] Conform With Python 3 Syntax #5154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fd26311
[cfggen] Conform With Python 3 Syntax
tahmed-dev Aug 11, 2020
21783ab
review comment
tahmed-dev Aug 12, 2020
4a723d9
review comments
tahmed-dev Aug 12, 2020
76ba1bf
fix build error
tahmed-dev Aug 12, 2020
2a255ec
2to3 missed sonic-cfggen script. Adding it.
tahmed-dev Aug 12, 2020
d14d923
review comment and rebasing to lastest.
tahmed-dev Aug 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions src/sonic-config-engine/config_samples.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def generate_t1_sample_config(data):
data['INTERFACE'] = {}
port_count = 0
total_port_amount = len(data['PORT'])
for port in natsorted(data['PORT'].keys()):
for port in natsorted(data['PORT']):
data['PORT'][port]['admin_status'] = 'up'
data['PORT'][port]['mtu'] = '9100'
local_addr = '10.0.{}.{}'.format(2 * port_count / 256, 2 * port_count % 256)
Expand All @@ -35,22 +35,22 @@ def generate_t1_sample_config(data):

def generate_empty_config(data):
new_data = {'DEVICE_METADATA': data['DEVICE_METADATA']}
if not new_data['DEVICE_METADATA']['localhost'].has_key('hostname'):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this necessary? https://www.tutorialspoint.com/python3/dictionary_has_key.htm

I can see python3 still have this support.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed. Not sure why 2to3 converted. I used 2to3 -f all switch. Are there recommended switches to use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in 3.1:

https://docs.python.org/3.1/whatsnew/3.0.html

Copy link
Contributor

@jleveque jleveque Aug 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm, has_key() has been removed in Python 3. in is the new proper way to check for a key.

if 'hostname' not in new_data['DEVICE_METADATA']['localhost']:
new_data['DEVICE_METADATA']['localhost']['hostname'] = 'sonic'
if not new_data['DEVICE_METADATA']['localhost'].has_key('type'):
if 'type' not in new_data['DEVICE_METADATA']['localhost']:
new_data['DEVICE_METADATA']['localhost']['type'] = 'LeafRouter'
return new_data

def generate_l2_config(data):
if not data['DEVICE_METADATA']['localhost'].has_key('hostname'):
if 'hostname' not in data['DEVICE_METADATA']['localhost']:
data['DEVICE_METADATA']['localhost']['hostname'] = 'sonic'
if not data['DEVICE_METADATA']['localhost'].has_key('type'):
if 'type' not in data['DEVICE_METADATA']['localhost']:
data['DEVICE_METADATA']['localhost']['type'] = 'ToRRouter'
data['VLAN'] = {'Vlan1000': {'vlanid': '1000'}}
vp = natsorted(data['PORT'].keys())
vp = natsorted(list(data['PORT'].keys()))
data['VLAN']['Vlan1000'].setdefault('members', vp)
data['VLAN_MEMBER'] = {}
for port in natsorted(data['PORT'].keys()):
for port in natsorted(data['PORT']):
data['PORT'][port].setdefault('admin_status', 'up')
data['VLAN_MEMBER']['Vlan1000|{}'.format(port)] = {'tagging_mode': 'untagged'}
return data
Expand All @@ -62,7 +62,7 @@ def generate_l2_config(data):
}

def get_available_config():
return _sample_generators.keys()
return list(_sample_generators.keys())
tahmed-dev marked this conversation as resolved.
Show resolved Hide resolved

def generate_sample_config(data, setting_name):
return _sample_generators[setting_name.lower()](data)
Expand Down
73 changes: 37 additions & 36 deletions src/sonic-config-engine/minigraph.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/usr/bin/env python
from __future__ import print_function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should not be necessary for backward-compatibility with Python 2.7, but it won't hurt anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was getting error related to the print function syntax:

    Running sonic-cfggen -y /sonic/src/sonic-config-engine/tests/test.yml -t /sonic/src/sonic-config-engine/tests/test.j2
-------------------------------------------------------------------------------------------------------------------------------------------------- Captured stderr call --------------------------------------------------------------------------------------------------------------------------------------------------
Traceback (most recent call last):
  File "/sonic/src/sonic-config-engine/tests/../sonic-cfggen", line 39, in <module>
    from minigraph import minigraph_encoder
  File "/sonic/src/sonic-config-engine/minigraph.py", line 291
    print("VNI must be an integer (use default VNI %d instead)" % vni_default, file=sys.stderr) 
                                                                                   ^
SyntaxError: invalid syntax

Copy link
Contributor

@jleveque jleveque Aug 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, maybe the file= argument was added in Python 3.0.

Edit: I figured out why from __future__ import print_function is not always needed in Python 2. In Python 2, adding parentheses around the arguments to print does not change it into a function call; it simply translates to "pass this tuple of args to the print statement". This works fine in Python 2, and in Python 3, it works because it is interpreted as a function call. However, adding the file= parameter won't work in Python 2 because print is not technically a function. This is the case when from __future__ import print_function is needed.

import calendar
import math
import os
Expand Down Expand Up @@ -124,13 +125,13 @@ def parse_png(png, hname):
bandwidth_node = link.find(str(QName(ns, "Bandwidth")))
bandwidth = bandwidth_node.text if bandwidth_node is not None else None
if enddevice.lower() == hname.lower():
if port_alias_map.has_key(endport):
if endport in port_alias_map:
endport = port_alias_map[endport]
neighbors[endport] = {'name': startdevice, 'port': startport}
if bandwidth:
port_speeds[endport] = bandwidth
elif startdevice.lower() == hname.lower():
if port_alias_map.has_key(startport):
if startport in port_alias_map:
startport = port_alias_map[startport]
neighbors[startport] = {'name': enddevice, 'port': endport}
if bandwidth:
Expand Down Expand Up @@ -175,14 +176,14 @@ def parse_asic_external_link(link, asic_name, hostname):
# if chassis internal is false, the interface name will be
# interface alias which should be converted to asic port name
if (enddevice.lower() == hostname.lower()):
if ((port_alias_asic_map.has_key(endport)) and
if ((endport in port_alias_asic_map) and
(asic_name.lower() in port_alias_asic_map[endport].lower())):
endport = port_alias_asic_map[endport]
neighbors[port_alias_map[endport]] = {'name': startdevice, 'port': startport}
if bandwidth:
port_speeds[port_alias_map[endport]] = bandwidth
elif (startdevice.lower() == hostname.lower()):
if ((port_alias_asic_map.has_key(startport)) and
if ((startport in port_alias_asic_map) and
(asic_name.lower() in port_alias_asic_map[startport].lower())):
startport = port_alias_asic_map[startport]
neighbors[port_alias_map[startport]] = {'name': enddevice, 'port': endport}
Expand All @@ -202,14 +203,14 @@ def parse_asic_internal_link(link, asic_name, hostname):
bandwidth = bandwidth_node.text if bandwidth_node is not None else None
if ((enddevice.lower() == asic_name.lower()) and
(startdevice.lower() != hostname.lower())):
if port_alias_map.has_key(endport):
if endport in port_alias_map:
endport = port_alias_map[endport]
neighbors[endport] = {'name': startdevice, 'port': startport}
if bandwidth:
port_speeds[endport] = bandwidth
elif ((startdevice.lower() == asic_name.lower()) and
(enddevice.lower() != hostname.lower())):
if port_alias_map.has_key(startport):
if startport in port_alias_map:
startport = port_alias_map[startport]
neighbors[startport] = {'name': enddevice, 'port': endport}
if bandwidth:
Expand Down Expand Up @@ -288,7 +289,7 @@ def parse_dpg(dpg, hname):
if vni_element.text.isdigit():
vni = int(vni_element.text)
else:
print >> sys.stderr, "VNI must be an integer (use default VNI %d instead)" % vni_default
print("VNI must be an integer (use default VNI %d instead)" % vni_default, file=sys.stderr)

ipintfs = child.find(str(QName(ns, "IPInterfaces")))
intfs = {}
Expand Down Expand Up @@ -391,18 +392,18 @@ def parse_dpg(dpg, hname):
# decide an ACL is a Control Plane ACL if acl_intfs is empty below.
for member in aclattach:
member = member.strip()
if pcs.has_key(member):
if member in pcs:
# If try to attach ACL to a LAG interface then we shall add the LAG to
# to acl_intfs directly instead of break it into member ports, ACL attach
# to LAG will be applied to all the LAG members internally by SAI/SDK
acl_intfs.append(member)
elif vlans.has_key(member):
elif member in vlans:
acl_intfs.append(member)
elif port_alias_map.has_key(member):
elif member in port_alias_map:
acl_intfs.append(port_alias_map[member])
# Give a warning if trying to attach ACL to a LAG member interface, correct way is to attach ACL to the LAG interface
if port_alias_map[member] in intfs_inpc:
print >> sys.stderr, "Warning: ACL " + aclname + " is attached to a LAG member interface " + port_alias_map[member] + ", instead of LAG interface"
print("Warning: ACL " + aclname + " is attached to a LAG member interface " + port_alias_map[member] + ", instead of LAG interface", file=sys.stderr)
elif member.lower().startswith('erspan') or member.lower().startswith('egress_erspan'):
if member.lower().startswith('erspanv6') or member.lower().startswith('egress_erspanv6'):
is_mirror_v6 = True
Expand Down Expand Up @@ -439,9 +440,9 @@ def parse_dpg(dpg, hname):
# append the service to our list of services
if aclname in acls:
if acls[aclname]['type'] != 'CTRLPLANE':
print >> sys.stderr, "Warning: ACL '%s' type mismatch. Not updating ACL." % aclname
print("Warning: ACL '%s' type mismatch. Not updating ACL." % aclname, file=sys.stderr)
elif acls[aclname]['services'] == aclservice:
print >> sys.stderr, "Warning: ACL '%s' already contains service '%s'. Not updating ACL." % (aclname, aclservice)
print("Warning: ACL '%s' already contains service '%s'. Not updating ACL." % (aclname, aclservice), file=sys.stderr)
else:
acls[aclname]['services'].append(aclservice)
else:
Expand All @@ -450,7 +451,7 @@ def parse_dpg(dpg, hname):
'stage': stage,
'services': [aclservice]}
except:
print >> sys.stderr, "Warning: Ignoring Control Plane ACL %s without type" % aclname
print("Warning: Ignoring Control Plane ACL %s without type" % aclname, file=sys.stderr)

return intfs, lo_intfs, mvrf, mgmt_intf, vlans, vlan_members, pcs, pc_members, acls, vni
return None, None, None, None, None, None, None, None, None, None
Expand Down Expand Up @@ -530,8 +531,8 @@ def parse_cpg(cpg, hname):
if hostname.lower() == bgp_session['name'].lower():
bgp_session['asn'] = asn

bgp_monitors = { key: bgp_sessions[key] for key in bgp_sessions if bgp_sessions[key].has_key('asn') and bgp_sessions[key]['name'] == 'BGPMonitor' }
bgp_sessions = { key: bgp_sessions[key] for key in bgp_sessions if bgp_sessions[key].has_key('asn') and int(bgp_sessions[key]['asn']) != 0 }
bgp_monitors = { key: bgp_sessions[key] for key in bgp_sessions if 'asn' in bgp_sessions[key] and bgp_sessions[key]['name'] == 'BGPMonitor' }
bgp_sessions = { key: bgp_sessions[key] for key in bgp_sessions if 'asn' in bgp_sessions[key] and int(bgp_sessions[key]['asn']) != 0 }

return bgp_sessions, myasn, bgp_peers_with_range, bgp_monitors

Expand Down Expand Up @@ -696,7 +697,7 @@ def parse_spine_chassis_fe(results, vni, lo_intfs, phyport_intfs, pc_intfs, pc_m
break

if intf_name == None:
print >> sys.stderr, 'Warning: cannot find any interfaces that belong to %s' % (pc_intf)
print('Warning: cannot find any interfaces that belong to %s' % (pc_intf), file=sys.stderr)
continue

# Get the neighbor router of this port channel interface
Expand Down Expand Up @@ -734,7 +735,7 @@ def filter_acl_table_bindings(acls, neighbors, port_channels, sub_role):
if not backend_port_channel:
front_port_channel_intf.append(port_channel_intf)

for acl_table, group_params in acls.iteritems():
for acl_table, group_params in acls.items():
group_type = group_params.get('type', None)
filter_acls[acl_table] = acls[acl_table]

Expand Down Expand Up @@ -763,7 +764,7 @@ def filter_acl_table_bindings(acls, neighbors, port_channels, sub_role):
active_ports = [port for port in front_panel_ports if port in neighbors.keys() or port in front_port_channel_intf]

if not active_ports:
print >> sys.stderr, 'Warning: mirror table {} in ACL_TABLE does not have any ports bound to it'.format(acl_table)
print('Warning: mirror table {} in ACL_TABLE does not have any ports bound to it'.format(acl_table), file=sys.stderr)

filter_acls[acl_table]['ports'] = active_ports

Expand Down Expand Up @@ -921,14 +922,14 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
results['BGP_PEER_RANGE'] = bgp_peers_with_range
if mgmt_routes:
# TODO: differentiate v4 and v6
mgmt_intf.itervalues().next()['forced_mgmt_routes'] = mgmt_routes
iter(mgmt_intf.values()).next()['forced_mgmt_routes'] = mgmt_routes
results['MGMT_PORT'] = {}
results['MGMT_INTERFACE'] = {}
mgmt_intf_count = 0
mgmt_alias_reverse_mapping = {}
for key in mgmt_intf:
alias = key[0]
if mgmt_alias_reverse_mapping.has_key(alias):
if alias in mgmt_alias_reverse_mapping:
name = mgmt_alias_reverse_mapping[alias]
else:
name = 'eth' + str(mgmt_intf_count)
Expand All @@ -953,14 +954,14 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
phyport_intfs = {}
vlan_intfs = {}
pc_intfs = {}
vlan_invert_mapping = { v['alias']:k for k,v in vlans.items() if v.has_key('alias') }
vlan_invert_mapping = { v['alias']:k for k,v in vlans.items() if 'alias' in v }
vlan_sub_intfs = {}

for intf in intfs:
if intf[0][0:4] == 'Vlan':
vlan_intfs[intf] = {}
vlan_intfs[intf[0]] = {}
elif vlan_invert_mapping.has_key(intf[0]):
elif intf[0] in vlan_invert_mapping:
vlan_intfs[(vlan_invert_mapping[intf[0]], intf[1])] = {}
vlan_intfs[vlan_invert_mapping[intf[0]]] = {}
elif intf[0][0:11] == 'PortChannel':
Expand All @@ -975,7 +976,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw

for port_name in port_speeds_default:
# ignore port not in port_config.ini
if not ports.has_key(port_name):
if port_name not in ports:
continue

ports.setdefault(port_name, {})['speed'] = port_speeds_default[port_name]
Expand All @@ -985,12 +986,12 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
# If no port_config_file is found ports is empty so ignore this error
if port_config_file is not None:
if port_name not in ports:
print >> sys.stderr, "Warning: ignore interface '%s' as it is not in the port_config.ini" % port_name
print("Warning: ignore interface '%s' as it is not in the port_config.ini" % port_name, file=sys.stderr)
continue

ports.setdefault(port_name, {})['speed'] = port_speed_png[port_name]

for port_name, port in ports.items():
for port_name, port in list(ports.items()):
# get port alias from port_config.ini
alias = port.get('alias', port_name)
# generate default 100G FEC
Expand All @@ -1001,34 +1002,34 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
# set port description if parsed from deviceinfo
for port_name in port_descriptions:
# ignore port not in port_config.ini
if not ports.has_key(port_name):
if port_name not in ports:
continue

ports.setdefault(port_name, {})['description'] = port_descriptions[port_name]

for port_name, port in ports.items():
if not port.get('description'):
if neighbors.has_key(port_name):
if port_name in neighbors:
# for the ports w/o description set it to neighbor name:port
port['description'] = "%s:%s" % (neighbors[port_name]['name'], neighbors[port_name]['port'])
else:
# for the ports w/o neighbor info, set it to port alias
port['description'] = port.get('alias', port_name)

# set default port MTU as 9100
for port in ports.itervalues():
for port in ports.values():
port['mtu'] = '9100'

# asymmetric PFC is disabled by default
for port in ports.itervalues():
for port in ports.values():
port['pfc_asym'] = 'off'

# set physical port default admin status up
for port in phyport_intfs:
if port[0] in ports:
ports.get(port[0])['admin_status'] = 'up'

for member in pc_members.keys() + vlan_members.keys():
for member in list(pc_members.keys()) + list(vlan_members.keys()):
port = ports.get(member[1])
if port:
port['admin_status'] = 'up'
Expand All @@ -1047,11 +1048,11 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
# remove portchannels that contain ports not existing in port_config.ini
# when port_config.ini exists
if not set(mbr_map['members']).issubset(port_set):
print >> sys.stderr, "Warning: ignore '%s' as part of its member interfaces is not in the port_config.ini" % pc_name
print("Warning: ignore '%s' as part of its member interfaces is not in the port_config.ini" % pc_name, file=sys.stderr)
del pcs[pc_name]

# set default port channel MTU as 9100 and admin status up
for pc in pcs.itervalues():
for pc in pcs.values():
pc['mtu'] = '9100'
pc['admin_status'] = 'up'

Expand All @@ -1061,7 +1062,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
for pc_intf in pc_intfs.keys():
# remove portchannels not in PORTCHANNEL dictionary
if isinstance(pc_intf, tuple) and pc_intf[0] not in pcs:
print >> sys.stderr, "Warning: ignore '%s' interface '%s' as '%s' is not in the valid PortChannel list" % (pc_intf[0], pc_intf[1], pc_intf[0])
print("Warning: ignore '%s' interface '%s' as '%s' is not in the valid PortChannel list" % (pc_intf[0], pc_intf[1], pc_intf[0]), file=sys.stderr)
del pc_intfs[pc_intf]
pc_intfs.pop(pc_intf[0], None)

Expand Down Expand Up @@ -1100,7 +1101,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw
# remove port not in port_config.ini
if nghbr not in ports:
if port_config_file is not None:
print >> sys.stderr, "Warning: ignore interface '%s' in DEVICE_NEIGHBOR as it is not in the port_config.ini" % nghbr
print("Warning: ignore interface '%s' in DEVICE_NEIGHBOR as it is not in the port_config.ini" % nghbr, file=sys.stderr)
del neighbors[nghbr]
results['DEVICE_NEIGHBOR'] = neighbors
if asic_name is None:
Expand Down Expand Up @@ -1198,4 +1199,4 @@ def parse_asic_sub_role(filename, asic_name):

def print_parse_xml(filename):
results = parse_xml(filename)
print(json.dumps(results, indent=3, cls=minigraph_encoder))
print((json.dumps(results, indent=3, cls=minigraph_encoder)))
Loading