changing the timeout of the binding files - #6018
Conversation
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 updates the configuration for OTG and gNMI connections within the sample binding files. By extending the timeout threshold, the changes ensure that test environments can better accommodate real-world deployment scenarios and avoid unnecessary failures caused by short timeout limits. 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 the 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 counterproductive. 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
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request increases the OTG and gNMI request timeouts from 30 seconds to 540 seconds in both topologies/otgdut_2.binding and topologies/otgdut_4.binding. The reviewer recommends reducing these timeouts to a more balanced value of 120 seconds to prevent excessively long test execution delays and CI/CD bottlenecks if services hang or become unreachable.
| target: "ixia-c-hostname:50051" # Change this to the Ixia-c-gnmi server endpoint. | ||
| skip_verify: true | ||
| timeout: 30 | ||
| timeout: 540 |
There was a problem hiding this comment.
Increasing the gNMI request timeout to 540 seconds (9 minutes) is excessively high. If a gNMI operation hangs, waiting 9 minutes will block the test suite and waste CI/CD resources. A timeout of 120 seconds is typically sufficient to handle transient delays while ensuring the test fails within a reasonable timeframe.
| timeout: 540 | |
| timeout: 120 |
There was a problem hiding this comment.
I insists the change because for most cases the API will return within a few seconds but for genuinely costly operations such as large learned info or stats , we need more time to respond, so a large timeout avoids false negatives and subsequent failures for subsequent tests
The OTG and gNMI timeout values specified in the sample binding files are currently too low to accommodate real-world deployment and test scenarios. We recommend increasing the maximum timeout to 9 minutes (540 seconds).
The existing 30-second timeout may not be sufficient under normal operating conditions and can result in misleading timeout failures, obscure error messages, and false alarms. A longer timeout will improve reliability and reduce the likelihood of reporting transient delays as test failures.