Xw/net 6307 grpc client apply #20107
Conversation
github-actions
bot
added
the
theme/cli
command/resource/apply-grpc/apply.go
Outdated
| if input == "" && len(c.flags.Args()) > 0 { | ||
| input = c.flags.Arg(0) | ||
| } | ||
| if input != "" { |
There was a problem hiding this comment.
You could eliminate some nesting/branching by having the flow be:
if input == "" {
// error out and return
}
// parse the input
command/resource/apply-grpc/apply.go
Outdated
| if input == "" && len(c.flags.Args()) > 0 { | ||
| input = c.flags.Arg(0) | ||
| } |
There was a problem hiding this comment.
My vote would be to not use positional args here but instead support only the following:
consul resource apply -f <path to file>- loads content from specified pathconsul resource apply -f -- loads content from stdin
This is how kubectl works and helps to reduce redundant ways that the input source can be specified.
In the future we could even support something like: consul resource apply -d <directory> to load all the files in the directory and apply them.
There was a problem hiding this comment.
Thanks for the input, now the code looks much much more cleaner!!! Thank you!!!
command/resource/apply-grpc/apply.go
Outdated
| input = c.flags.Arg(0) | ||
| } | ||
| if input != "" { | ||
| data, err := resource.ParseResourceInput(input, c.testStdin) |
There was a problem hiding this comment.
nit: You could do parsedResource, err = ... here and avoid the intermediate data variable.
command/resource/apply-grpc/apply.go
Outdated
| // parse resource | ||
| input := c.filePath | ||
| if input == "" { | ||
| c.UI.Error("Incorrect argument format: Must provide exactly one positional argument to specify the resource to write") |
There was a problem hiding this comment.
| c.UI.Error("Incorrect argument format: Must provide exactly one positional argument to specify the resource to write") | |
| c.UI.Error("Required '-f' flag was not provided to specify where to load the resource content from") |
There was a problem hiding this comment.
Thanks for the catch!
wangxinyi7
added
pr/no-changelog
Description
In this PR, I enabled the apply command in grpc communication. I created all of the code in separate files to avoid break changes to the current HTTP communication. We will purge all of the deprecated HTTP legacy after we completely support the grpc.
The most important changes are in these two files:
command/resource/apply-grpc/apply.goThis is the
Runcommand which executes theapplycall in CLIcommand/resource/resource-grpc.goThis is abstract of the underlying
APPLYcall. In the future, theLISTDELETEandREADwill all reside in this file.Currently they all have
-grpcin the file name to avoid duplication to the current files. As I mentioned above we will change the files names after we settle down everything.This PR is based off this PR which adds the
tokenfield to the CLI.PR Checklist
appropriate backport labels added