[cli] add lease info to srp server host and srp server service - #12589
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 significantly improves the visibility into the SRP server's lease management by extending the CLI output for hosts and services to include comprehensive lease and remaining lease details. This enhancement provides users with critical timing information directly from the command line, facilitating better monitoring and debugging of SRP server operations. The changes also ensure that automated testing and external tools can accurately parse and utilize this new data. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request enhances the srp server host and srp server service CLI commands by adding lease and remaining lease information to their output. A new utility method OutputMsecDurationInSec is introduced for formatting time durations. The changes are well-implemented, and the corresponding test parsers in Python are updated to accommodate the new fields. My review includes a couple of suggestions to improve the robustness of the test parsers.
Library files
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12589 +/- ##
==========================================
+ Coverage 75.30% 75.50% +0.20%
==========================================
Files 696 695 -1
Lines 96239 94061 -2178
==========================================
- Hits 72469 71020 -1449
+ Misses 23770 23041 -729
🚀 New features to boost your workflow:
|
f3ef881 to
a9035ab
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances the SRP server CLI by adding lease and remaining lease information to the srp server host and srp server service commands. A new utility function is introduced for formatting time durations, and the relevant test and tool parsers are updated to accommodate these new fields. The changes are well-structured, but I found a critical syntax error in one of the test script parsers that needs to be fixed.
Note: Security Review is unavailable for this PR.
a9035ab to
f62f25e
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request is well-structured, adding lease and remaining lease information to the srp server host and srp server service CLI commands. The C++ changes are clean, introducing a helper function OutputLeaseInfo to avoid code duplication and improve maintainability. The documentation and test scripts are updated accordingly to reflect these changes. I've added a few suggestions in the Python test scripts to make the parsing logic more robust.
I am having trouble creating individual review comments. Click here to see my feedback.
tests/scripts/thread-cert/node.py (1182-1183)
Using split(':', 1) is more robust for parsing key-value pairs, as it ensures that only the first colon is used as a delimiter. This prevents potential parsing errors if the value itself contains a colon.
key_value = lines.pop(0).strip().split(':', 1)
host[key_value[0].strip()] = key_value[1].strip()
tests/scripts/thread-cert/node.py (1193-1194)
Using split(':', 1) is more robust for parsing key-value pairs, as it ensures that only the first colon is used as a delimiter. This prevents potential parsing errors if the value itself contains a colon.
key_value = lines.pop(0).strip().split(':', 1)
host[key_value[0].strip()] = key_value[1].strip()
tests/scripts/thread-cert/node.py (1254-1255)
Using split(':', 1) is more robust for parsing key-value pairs, as it ensures that only the first colon is used as a delimiter. This prevents potential parsing errors if the value itself contains a colon.
key_value = lines.pop(0).strip().split(':', 1)
service[key_value[0].strip()] = key_value[1].strip()
tests/scripts/thread-cert/node.py (1262-1263)
Using split(':', 1) is more robust for parsing key-value pairs, as it ensures that only the first colon is used as a delimiter. This prevents potential parsing errors if the value itself contains a colon.
key_value = lines.pop(0).strip().split(':', 1)
service[key_value[0].strip()] = key_value[1].strip()
tests/toranj/cli/cli.py (742-743)
Using split(':', 1) is more robust for parsing key-value pairs, as it ensures that only the first colon is used as a delimiter. This prevents potential parsing errors if the value itself contains a colon.
key_value = outputs.pop(0).strip().split(':', 1)
host[key_value[0].strip()] = key_value[1].strip()
tests/toranj/cli/cli.py (751-752)
Using split(':', 1) is more robust for parsing key-value pairs, as it ensures that only the first colon is used as a delimiter. This prevents potential parsing errors if the value itself contains a colon.
key_value = outputs.pop(0).strip().split(':', 1)
host[key_value[0].strip()] = key_value[1].strip()
tests/toranj/cli/cli.py (804-805)
Using split(':', 1) is more robust for parsing key-value pairs, as it ensures that only the first colon is used as a delimiter. This prevents potential parsing errors if the value itself contains a colon.
key_value = outputs.pop(0).strip().split(':', 1)
service[key_value[0].strip()] = key_value[1].strip()
tests/toranj/cli/cli.py (811-812)
Using split(':', 1) is more robust for parsing key-value pairs, as it ensures that only the first colon is used as a delimiter. This prevents potential parsing errors if the value itself contains a colon.
key_value = outputs.pop(0).strip().split(':', 1)
service[key_value[0].strip()] = key_value[1].strip()
This commit adds lease and remaining lease information to the output of `srp server host` and `srp server service` CLI commands. The information includes: - `lease`: The total lease time in seconds. - `key-lease`: The total key lease time in seconds. - `remaining lease`: The remaining lease time in seconds (with millisecond precision). - `remaining key-lease`: The remaining key lease time in seconds (with millisecond precision). A new utility method `OutputMsecDurationInSec()` is added to `Utils` class to format durations in milliseconds as seconds with a fractional part. The SRP server host and service output parsers in `tests/scripts/thread-cert/node.py`, `tests/toranj/cli/cli.py`, and `tools/otci/otci/otci.py` are updated to correctly handle the new fields for both active and deleted entries.
f62f25e to
d96549a
Compare
This commit adds lease and remaining lease information to the output of
srp server hostandsrp server serviceCLI commands.The information includes:
lease: The total lease time in seconds.key-lease: The total key lease time in seconds.remaining lease: The remaining lease time in seconds (with millisecond precision).remaining key-lease: The remaining key lease time in seconds (with millisecond precision).A new utility method
OutputMsecDurationInSec()is added toUtilsclass to format durations in milliseconds as seconds with a fractional part.The SRP server host and service output parsers in
tests/scripts/thread-cert/node.py,tests/toranj/cli/cli.py, andtools/otci/otci/otci.pyare updated to correctly handle the new fields for both active and deleted entries.