Conversation
0d1e799 to
a55cee8
Compare
| return nil, false | ||
| } | ||
|
|
||
| cmd := exec.CommandContext(ctx, extPath, extArgs...) |
There was a problem hiding this comment.
Interrupt signal handling was called out in the CLI extension proposal as an area of active search; here we decide to go with the option of aborting the sub-command on interrupt to unblock this work. This will be re-evaluated after gathering more data.
There was a problem hiding this comment.
This may be good enough. On nix-based platforms, I think we should consider tunnelling signals (e.g. sigint) and on context timeout, sending sigterm in this case instead of sigkill, but on Windows hard kill like this is acceptable. We will need to test I think.
e4f973d to
bddc00b
Compare
bddc00b to
857254c
Compare
| // | ||
| // If temporal-foo-bar_baz is found, returns (path, ["--flag=value"]). | ||
| // If temporal-foo is found, returns (path, ["--flag=value", "bar-baz"]). | ||
| func lookupExtension(args []string) (path string, extArgs []string) { |
There was a problem hiding this comment.
The CLI extension proposal describes the question of "whether flags of parent commands can still be placed before the extension command" as an area of active research. For now we require them to be placed after the extension command to disambiguate. This will be re-evaluated after gathering more data.
There was a problem hiding this comment.
I think you should find the most applicable built-in sub-command, remove all common flags on that subcommand, then do your extension resolution (and pass along all of those flags to the subcommand). And you should fail if there are unrecognized parent/common flags before the subcommand. For instance, I would expect:
temporal --output json my-command --some-bool-argto runtemporal-something --output json --some-bool-argtemporal --no-json-shorthand-payloads my-commandto runtemporal-my-command --no-json-shorthand-payloadstemporal --address 1.2.3.4:5678 workflow --tls diagramto runtemporal-workflow-diagram --address 1.2.3.4:5678 --tlstemporal workflow --some-bool-arg diagramto fail with "unrecognized flag --some-bool-arg"temporal --some-arg some-value workflow diagramto fail with "unrecognized flag --some-arg"
I have not investigated how to do this yet or how much of it is possible, but you cannot comfortably know which flags accept values and which are standalone to do some kind of arbitrary parsing. May have to put a ArbitraryArgs on every command. I can help with a PoC of this if needed.
18e1453 to
2ff2d54
Compare
2ff2d54 to
5c9544e
Compare
1edb099 to
4f129e8
Compare
635b21c to
165ca33
Compare
165ca33 to
0285f93
Compare
cretz
left a comment
There was a problem hiding this comment.
Looks great. I appreciate the test building an executable. I assume there are no backwards incompatibilities for users today.
1041e26 to
10a279d
Compare
10a279d to
eadb2cd
Compare
What was changed
CLI now invokes extensions in accordance with CLI Extensions proposal.
Note that the help command integration part is going to be in a follow-up PR.
Why?
Allow Temporal CLI Extensions.
Checklist
Closes
How was this tested: