gNMI-1.11 Add deviation to skip unsupported paths.#5284
gNMI-1.11 Add deviation to skip unsupported paths.#5284AnilKR123 wants to merge 1 commit intoopenconfig:mainfrom
Conversation
Pull Request Functional Test Report for #5284 / c19410cVirtual Devices
Hardware Devices
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new deviation mechanism to allow for the skipping of specific unsupported subinterface state counter paths on Arista hardware. By isolating these counters from the existing general packet counter deviation, the changes ensure that supported IPv4/IPv6 counters remain tested while correctly bypassing unsupported paths. Additionally, the PR cleans up the test suite by removing an irrelevant L1 carrier transition check. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new deviation, default_subinterface_packet_counters_missing, to handle cases where devices lack specific subinterface state packet counters. The changes include updating the metadata proto, adding an accessor in the deviations package, and modifying the telemetry interface packet counters test to utilize this new deviation for skipping relevant test cases. Feedback indicates that the new deviation accessor in internal/deviations/deviations.go is missing a required issue tracker URL in its documentation, which is a violation of the repository style guide.
| // DefaultSubinterfacePacketCountersMissing returns if device is missing subinterface state packet counters. | ||
| // Default value is false. | ||
| func DefaultSubinterfacePacketCountersMissing(dut *ondatra.DUTDevice) bool { | ||
| return lookupDUTDeviations(dut).GetDefaultSubinterfacePacketCountersMissing() | ||
| } |
There was a problem hiding this comment.
The deviation accessor function DefaultSubinterfacePacketCountersMissing is missing a URL link to an issue tracker. According to the Repository Style Guide (Section 2, Step 5), every deviation accessor must include a comment with a URL (e.g., https://issuetracker.google.com/issues/xxxxx) tracking its removal.
References
- Add a comment to the accessor function containing a URL link to an issue tracker which tracks removal of the deviation. (link)
Adding a deviation to skip the following unsupported subinterface state counter paths for the default subinterface on Arista (subinterface index 0):
The existing subinterface_packet_counters_missing deviation was not used as it also skips IPv4/IPv6 counters which are supported on Arista.
The following paths are confirmed working and remain tested on Arista (subinterface index 0):
I also removed carrier-transitions from the subinterface counter test as it is an L1 concept and does not apply to a subinterface.