feat: OC to IOS-XE QoS by jrouliez · Pull Request #246 · model-driven-devops/nso-oc-services

jrouliez · 2023-07-03T03:15:29Z

feat: OC to IOS-XE QoS

…ven-devops/nso-oc-services into oc_to_xe_qos_class_map

m0lass3s · 2023-07-05T21:46:48Z

mdd/python/translation/openconfig_xe/xe_qos.py

+        raise ValueError(
+            f'Interface type {interface_type} not supported by this NSO_OC_Services implementation. Please file an issue at https://github.com/model-driven-devops/nso-oc-services')
+
+def modify_dscp(dscp):


Instead of a long winded if-else statement, just create a globally scoped object. We can then map by key-value pairs, where keys are dscp and values are those currently in new_dscp. You can still account for your if, but your else if and else can be converted to use the object.

Updates made.

m0lass3s · 2023-07-05T21:46:52Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    """
+    Configure forwarding-groups
+    """
+    if len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) > 0:


This if statement isn't required. The loop will run if there are more than 0 elements.

SIDE NOTE: I am not aware if nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group is guaranteed to always return something (i.e. it's defined). If so, then ignore this side note. This note refers to everything under nso_props.service.oc_qos__qos.

Changing block to:
for fg in nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group:
device_cdb.ios__policy_map.create(fg.name)
if len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) == 0:
if device_cdb.ios__policy_map:
device_cdb.ios__policy_map.delete()

m0lass3s · 2023-07-05T21:46:54Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    if len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) > 0:
+        for fg in nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group:
+            device_cdb.ios__policy_map.create(fg.name)
+    elif len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) == 0:


Change to an if statement, since we're getting rid of the if statement above. It will only run if forwarding_group is empty.

See the reply from the previous conversation.

m0lass3s · 2023-07-05T21:52:14Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    """
+    Configure schedulers
+    """
+    if len(nso_props.service.oc_qos__qos.scheduler_policies.scheduler_policy) > 0:


This if statement isn't required. The loop will run if there are more than 0 elements.

'if' statement removed.

m0lass3s · 2023-07-05T21:53:43Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    """
+    Configure classifiers
+    """
+    if len(nso_props.service.oc_qos__qos.classifiers.classifier) > 0:


This if statement isn't required. The loop will run if there are more than 0 elements.

'if' statement removed.

m0lass3s · 2023-07-05T21:54:18Z

mdd/python/translation/openconfig_xe/xe_qos.py

+                        device_cdb.ios__class_map[c_map.name].prematch = 'match-all'
+                        device_cdb.ios__class_map[c_map.name].match.dscp.create(
+                            new_dscp)
+    elif len(nso_props.service.oc_qos__qos.classifiers.classifier) == 0:


Change to an if statement, since we're getting rid of the if statement above.

Updated 'elif' to 'if'.

m0lass3s · 2023-07-05T21:55:20Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    """
+    if len(nso_props.service.oc_qos__qos.scheduler_policies.scheduler_policy) > 0:
+        for sched_pol in nso_props.service.oc_qos__qos.scheduler_policies.scheduler_policy:
+            if len(sched_pol.schedulers.scheduler) > 0:


This if statement isn't required. The loop will run if there are more than 0 elements.

'if' statement removed.

m0lass3s · 2023-07-05T22:00:26Z

mdd/python/translation/openconfig_xe/xe_qos.py

+                for sequence in sched_pol.schedulers.scheduler:
+                    # Configure policy-map
+                    p_map = device_cdb.ios__policy_map.create(sequence.output.config.output_fwd_group)
+                    if pmap_cmap:


Combine this to a single line, to prevent unnecessary tabbing

Suggested change

if pmap_cmap:

if pmap_cmap and p_map.name in pmap_cmap.keys():

'if' statement combined.

m0lass3s · 2023-07-05T22:02:29Z

mdd/python/translation/openconfig_xe/xe_qos.py

+                    """
+                    Configure interfaces
+                    """
+                    if len(nso_props.service.oc_qos__qos.interfaces.interface) > 0:


This if statement isn't required. The loop will run if there are more than 0 elements.

'if' statement removed.

m0lass3s · 2023-07-05T22:24:13Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    if sequence.one_rate_two_color:
+        # Configure cir percentage
+        if sequence.one_rate_two_color.config.cir_pct:
+            device_cdb.ios__policy_map[p_map].ios__class[c_map].police_cir_percent.police.cir.percent.percentage = sequence.one_rate_two_color.config.cir_pct


The code can be cleaned up to make it less verbose. We can greatly remove a lot of repeated text such as device_cdb.ios__policy_map[p_map].ios__class[c_map].police_cir_percent.police.cir.percent by storing them in variables and referencing those variables instead of the long text. device_cdb.ios__policy_map[p_map].ios__class[c_map].police_cir_percent.police.cir.percent is just an example and there are a few others.

Updates made.

m0lass3s · 2023-07-12T21:31:14Z

mdd/python/translation/openconfig_xe/xe_qos.py

+def modify_dscp(dscp):
+    if (dscp % 2) != 0:
+        return dscp
+    if dscp in dscp_dict.keys():


This if statement isn't required. The get function will perform a check if that key exist, otherwise that second arg ('default') value is returned.

Suggested change

if dscp in dscp_dict.keys():

return dscp_dict.get(dscp, 'default')

If statement removed.

m0lass3s · 2023-07-12T21:40:40Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    if len(nso_props.service.oc_qos__qos.classifiers.classifier) == 0:
+        if device_cdb.ios__class_map:


Minor, but you can just combine these two if statements since you're checking both for truthy.

Suggested change

if len(nso_props.service.oc_qos__qos.classifiers.classifier) == 0:

if device_cdb.ios__class_map:

if (len(nso_props.service.oc_qos__qos.classifiers.classifier) == 0

and device_cdb.ios__class_map):

m0lass3s · 2023-07-12T21:41:28Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    if len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) == 0:
+        if device_cdb.ios__policy_map:


Minor, but you can just combine these two if statements since you're checking both for truthy.

Suggested change

if len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) == 0:

if device_cdb.ios__policy_map:

if (len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) == 0

and device_cdb.ios__policy_map):

stmosher-dev

run tests

rerun

stmosher-dev

LGTM

rerun

jrouliez added 7 commits June 6, 2023 19:36

feat: OC to XE QoS Class-map

735f3dc

Merge branch 'main' into oc_to_xe_qos_class_map

181edb3

feat: OC to XE QoS Policy-map, One rate two color, Two rate three color.

fc3ee6d

Merge branch 'main' into oc_to_xe_qos_class_map

caeb439

Merge branch 'oc_to_xe_qos_class_map' of https://github.com/model-dri…

0d66255

…ven-devops/nso-oc-services into oc_to_xe_qos_class_map

feat: OC to XE QoS Two rate three color.

0efb147

feat: OC to XE QoS main updates

a8cbd76

m0lass3s requested changes Jul 6, 2023

View reviewed changes

feat: update to support mutiple class-maps in policy-maps

a34d8de

jrouliez force-pushed the oc_to_xe_qos_class_map branch from 0666e8b to a34d8de Compare July 12, 2023 17:20

m0lass3s reviewed Jul 12, 2023

View reviewed changes

jrouliez force-pushed the oc_to_xe_qos_class_map branch from fec7a74 to 40cda2d Compare July 13, 2023 04:04

stmosher-dev previously approved these changes Jul 13, 2023

View reviewed changes

jrouliez force-pushed the oc_to_xe_qos_class_map branch 2 times, most recently from c3140df to 84181b3 Compare July 14, 2023 04:41

stmosher-dev previously approved these changes Jul 14, 2023

View reviewed changes

fix: update classifier object to account for 'class-default'.

2e25df8

jrouliez force-pushed the oc_to_xe_qos_class_map branch from 84181b3 to 2e25df8 Compare July 14, 2023 19:03

stmosher-dev approved these changes Jul 17, 2023

View reviewed changes

	if pmap_cmap:
	if pmap_cmap and p_map.name in pmap_cmap.keys():

	if dscp in dscp_dict.keys():
	return dscp_dict.get(dscp, 'default')

		if len(nso_props.service.oc_qos__qos.classifiers.classifier) == 0:
		if device_cdb.ios__class_map:

		if len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) == 0:
		if device_cdb.ios__policy_map:

Conversation

jrouliez commented Jul 3, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrouliez Jul 7, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

m0lass3s Jul 12, 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!

Choose a reason for hiding this comment

Uh oh!

m0lass3s Jul 12, 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!

Choose a reason for hiding this comment

Uh oh!

m0lass3s Jul 12, 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!

Choose a reason for hiding this comment

Uh oh!

stmosher-dev left a comment

Choose a reason for hiding this comment

Uh oh!

stmosher-dev left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jrouliez Jul 7, 2023 •

edited

Loading

m0lass3s Jul 12, 2023 •

edited

Loading

m0lass3s Jul 12, 2023 •

edited

Loading

m0lass3s Jul 12, 2023 •

edited

Loading