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.

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

Routed subinterface enhancements #8761

Merged
merged 7 commits into from Nov 29, 2021
Merged

Conversation

preetham-singh
Copy link
Contributor

Why I did it

Routed subinterfae enhancements HLD #833
Adding python API support to get routed subinterface long name to get correct parent interface for the routed subinterface.

How I did it

Adding python API.

How to verify it

Configuring routed subinterface on Physical & port channel subinterfaces.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

A picture of a cute animal (not mandatory but encouraged)

@hasan-brcm
Copy link
Contributor

/azpw run

@zhangyanzhao
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@zhangyanzhao
Copy link
Collaborator

HLD is not merged yet, we should merge the HLD before merging this code PR.

@prsunny
Copy link
Contributor

prsunny commented Oct 29, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

if intf is None:
return None

if VLAN_SUB_INTERFACE_SEPARATOR in intf:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to check Eth or Po prefix as well instead of blindly checking for "." in the intf string and returning long interface name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In certain case we need to fetch full name for the given subinterface parent interface which could be in short name format. Just checking for "." will not suffice here.

@lgtm-com
Copy link

lgtm-com bot commented Nov 9, 2021

This pull request introduces 1 alert when merging 60500f345691633fd466ca40592c57c5680adf8d into 07038a0 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@adyeung
Copy link
Collaborator

adyeung commented Nov 16, 2021

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 8761 in repo Azure/sonic-buildimage

@prsunny
Copy link
Contributor

prsunny commented Nov 16, 2021

@lgtm-com
Copy link

lgtm-com bot commented Nov 16, 2021

This pull request introduces 1 alert when merging 2e7b38e50f7a7dfd467e12ba32d8f82f506d9fb3 into bb798a3 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

else:
return str(intf)

def intf_get_longname(intf):
Copy link
Contributor

Choose a reason for hiding this comment

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

please follow a similar naming convention in the file.. suggest rename to get_subintf_name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this is being used to get longname for both subinterface and parent of subinterface, I have updated to use get_intf_longname as part of next commit.

prsunny
prsunny previously approved these changes Nov 17, 2021
@prsunny
Copy link
Contributor

prsunny commented Nov 19, 2021

/AzurePipelines run Azure.sonic-buildimage,

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sonic-net sonic-net deleted a comment from azure-pipelines bot Nov 19, 2021
@prsunny
Copy link
Contributor

prsunny commented Nov 22, 2021

@preetham-singh , could you please rebase?

@lgtm-com
Copy link

lgtm-com bot commented Nov 23, 2021

This pull request introduces 1 alert when merging 7b2237e9385c6ce3f76caf0f6bfb3c692c8fa7cf into 5f235a9 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@prsunny
Copy link
Contributor

prsunny commented Nov 23, 2021

looks like incorrect rebase. Could you please fix?

@lgtm-com
Copy link

lgtm-com bot commented Nov 24, 2021

This pull request introduces 1 alert when merging 0626347bc424e9dbde6f87484a293023e4c4821d into 11a93d2 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented Nov 24, 2021

This pull request introduces 1 alert when merging 56e8236 into b3ccef9 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@prsunny prsunny merged commit 858f430 into sonic-net:master Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants