-
Notifications
You must be signed in to change notification settings - Fork 848
traffic_ctl: Add --append option to append debug tags instead of replacing them. (inc ArgParser support).
#12804
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
base: master
Are you sure you want to change the base?
Conversation
…le usages - Add with_required() method to specify that an option requires another - Change add_example_usage() to support multiple examples per command - Add unit tests for option dependencies - Update ArgParser documentation
… tags - Add --append/-a option to append debug tags instead of replacing them - Uses ArgParser's with_required() to enforce --tags dependency - Add autest for server debug enable/disable commands - Update traffic_ctl documentation
--append option to append debug tags instead of replacing them. (inc ArgParser support).
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.
Pull request overview
This PR adds a new --append option to the traffic_ctl server debug enable command that allows appending debug tags to existing tags instead of replacing them. It also enhances the ArgParser library with option dependency support via a new with_required() method and updates add_example_usage() to support multiple examples per command.
Changes:
- Added
with_required()method to ArgParser for specifying option dependencies - Modified
add_example_usage()to support multiple example usage strings - Implemented
--append/-aflag intraffic_ctl server debug enablecommand with proper dependency on--tags
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| include/tscore/ArgParser.h | Added with_required() method declaration, changed _example_usage to _example_usages vector, added dependency tracking fields |
| src/tscore/ArgParser.cc | Implemented with_required() and validate_dependencies() methods, updated example usage handling, added dependency info to help output |
| src/tscore/unit_tests/test_ArgParser_OptionDependencies.cc | Comprehensive unit tests for option dependency feature |
| src/tscore/CMakeLists.txt | Added new unit test file to build |
| src/traffic_ctl/traffic_ctl.cc | Added --append option with with_required("--tags") and multiple example usages |
| src/traffic_ctl/CtrlCommands.h | Added APPEND_STR constant |
| src/traffic_ctl/CtrlCommands.cc | Implemented tag appending logic in server_debug() |
| tests/gold_tests/traffic_ctl/traffic_ctl_test_utils.py | Added Debug class for testing debug commands |
| tests/gold_tests/traffic_ctl/traffic_ctl_server_debug.test.py | New autest for debug enable/disable with append functionality |
| tests/gold_tests/traffic_ctl/gold/test_2.gold | Removed (no longer needed) |
| tests/gold_tests/traffic_ctl/gold/test_3.gold | Removed (no longer needed) |
| doc/developer-guide/internal-libraries/ArgParser.en.rst | Documented option dependencies feature |
| doc/appendices/command-line/traffic_ctl.en.rst | Documented --append option with examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ArgParser
Changes needed to implement the
traffic_ctlfeature.with_required()method to specify that an option requires anotheradd_example_usage()to support multiple examples per commandtraffic_ctl server debug: new
--append/-aoption when setting tags.Current implementation wipes what's on the debug tags and set the new one, we now have the option to append to the existing tags.
-append/-aoption to append debug tags instead of replacing themwith_required()to enforce--tagsdependencyenable/disablecommandstraffic_ctldocumentationThere are some cleanup as well, as this removes two gold files that aren't required as they are generated by the test.