-
Notifications
You must be signed in to change notification settings - Fork 3
Command Level Templating #264
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: main
Are you sure you want to change the base?
Conversation
f8eb458 to
eaefaee
Compare
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 introduces command-level templating functionality, allowing commands to declare named arguments that can be distributed to modules via ${arg-name} template syntax. The changes refactor the "main module" concept to "interactive module" to better reflect its purpose, and make having an interactive module optional when using command-level arguments.
Key changes:
- Adds
Argsfield toCommandstruct for declaring named command arguments with positional mapping - Renames
MaintoInteractiveinModuleConfigto clarify the module's role - Implements template substitution using
os.Expandwith validation of template references during configuration parsing
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/dut/dut.go | Core implementation: adds ArgDecl struct, template validation and substitution logic, renames Main to Interactive, updates error definitions |
| pkg/dut/dut_test.go | Comprehensive test coverage for template validation, extraction, and substitution with various edge cases |
| cmds/dutagent/states.go | Refactors module execution: adds getModuleArgs for unified argument distribution, extracts runModules function, adds runtime validation for argument requirements |
| cmds/dutagent/states_test.go | Updates test cases to use Interactive instead of Main, renames variables for consistency |
| cmds/dutagent/rpc.go | Updates buildCommandHelp to handle commands without interactive modules and append command args documentation |
| docs/dutagent-config.md | Updates schema documentation to reflect interactive field and new command-level args capability |
| docs/command-arg-templating.md | New comprehensive guide explaining three argument approaches: non-interactive, interactive, and templating with examples |
| contrib/dutagent-cfg-example.yaml | Demonstrates new templating feature with file-transfer command example |
| pkg/module/*/*-example-cfg.yml | Updates all module examples from main: true to interactive: true |
| cmds/exp/contrib/config-*.yaml | Updates experimental config files to use interactive instead of main |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
86ab59d to
f3754a7
Compare
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
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
f3754a7 to
346a3aa
Compare
|
@jenstopp One thing i noticed when working on this is that the |
346a3aa to
5ef97b3
Compare
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
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
llogen
left a comment
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.
I have nothing to complain about. Looks very clean and straight forward to me
|
Tested this on the internal board with an updated config using all methods. |
76cb0db to
95a4824
Compare
95a4824 to
f0de77e
Compare
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
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Extract args distribution logic into getModuleArgs function and pass runtime args explicitly to runModules goroutine, creating a single unified point for args consumption. Signed-off-by: Fabian Wienand <fabian.wienand@9elements.com>
Main modules are now optional. Commands can have 0 or 1 main modules instead of requiring exactly 1.Arguments are only passed to commands that have a main module else they are ignored with a validation error. Signed-off-by: Fabian Wienand <fabian.wienand@9elements.com>
Updated terminology from "main module" to "interactive module" to better reflect the module's purpose of receiving runtime arguments. Signed-off-by: Fabian Wienand <fabian.wienand@9elements.com>
f0de77e to
44a2072
Compare
Commands can now declare named arguments and distribute them to modules
using ${argname} template syntax. This enables sharing arguments across
multiple modules in a command.
Signed-off-by: Fabian Wienand <fabian.wienand@9elements.com>
44a2072 to
4bd25a5
Compare
This PR introduces command level templating as discussed in #248. The first commit refactors the dutagent with a common entry point for passing args to modules. The second commit makes the concept of a main module optional. The third commit adds command level templating as seen above. The help texts are also improved but still need to be refactored after #191 is resolved.