-
Notifications
You must be signed in to change notification settings - Fork 664
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
[debug dump util] Route Module added #1913
Conversation
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
…o dump_module_route
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
…o dump_module_route
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
This pull request introduces 1 alert when merging 8b9228b into 563c416 - view on LGTM.com new alerts:
|
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
@prsunny @SuvarnaMeenakshi Can you please review? |
@TACappleman - would you please review? Thanks. |
self.init_asic_nh() | ||
return self.ret_temp | ||
|
||
def add_to_ret_template(self, table, db, keys, err, add_to_tables_not_found=True): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be made a method on the Executor class, to avoid needing to redefine each time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's another such refactoring request here: #1853 (comment),
I'll also do this after once the Route and VxLan modules are merged. It'll be cleaner that way
dump/plugins/route.py
Outdated
return key_dict.get("vr", "") | ||
|
||
|
||
class NextHopGroupMatchOptimizer(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of this is generic code that may be useful for other plugins in future (you're already also using it for VRF IDs) so put in a common place
return "*\"dest\":\"" + dest + "\"*" | ||
|
||
|
||
def get_vr_oid(asic_route_entry): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment to document expectations of route entry format
…o dump_module_route
Signed-off-by: Vivek Reddy Karri <vkarri@nvidia.com>
…onic-utilities into dump_module_route
This pull request introduces 1 alert when merging ac92cdd into ac8382f - view on LGTM.com new alerts:
|
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
@TACappleman, I've addressed the comments. Please review them |
Signed-off-by: Vivek Reddy Karri vkarri@nvidia.com
What I did
HLD for Dump Utility: HLD.
How I did it
How to verify it
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)