Skip to content
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

Execute same command concurrently #908

Open
mustafaakin opened this issue Jul 15, 2019 · 11 comments
Open

Execute same command concurrently #908

mustafaakin opened this issue Jul 15, 2019 · 11 comments
Labels
area/cobra-command Core `cobra.Command` implementations kind/feature A feature request for cobra; new or enhanced behavior lifecycle/needs-proposal For large features/bugs that need a proposal and community buy in

Comments

@mustafaakin
Copy link

I'm building an application that will listen text messages through slack and pass it through to CLI to execute messages. However, as the command object holds contextual data such as args, I'm not sure how can I reuse it concurrently safely.

The obvious solution would be to extract building the root command to a function so every time it is called a new struct is generated. Do you recommend such usage or is there any way that I cannot find?

My current usage is like the following:

var rootCmd = &cobra.Command{
	Use:   "app",
	Short: "My cool app",
	Run: func(cmd *cobra.Command, args []string) {
	},
}

var versionCmd = &cobra.Command{
	Use:   "version",
	Short: "Print the version number",
	Run: func(cmd *cobra.Command, args []string) {
		fmt.Fprint(cmd.OutOrStdout(), "1.0")
	},
}

func init() {
	rootCmd.AddCommand(versionCmd)
}

func main() {
	rootCmd.SetArgs([]string{"version"})
	rootCmd.SetOut(os.Stdout) // Ideally a buffer
	err := rootCmd.Execute()
	fmt.Println(err)
}
@jharshman
Copy link
Collaborator

Why do you want to pass the Slack messages to the CLI? It sounds like you want to build a Slack bot in which case there is a lot of documentation online on how to accomplish this.

In short, your application is going to want to listen for slack message events and then do something for that event. From that point, concurrently handling events is a pretty simple addition.

@mustafaakin
Copy link
Author

Thanks for the input but you are not answering the question. Get message and do something is very generic. Besides being submitted by Slack, most of the functions are just same text commands that cobra parses by using os.Args[1:].

My Slackbot is simple, not contextual or anything, like writing kubectl get -n kube-system pod works in terminal, it would work much of the same with Slack messages. I fail to see why would I manually string parse a Slack submitted message string, although this great library exists.

I am generating cobra.Command structure each time a new message comes to avoid concurrency, I was asking if I should continue to do that or is there any feature that allows using Cobra concurrently? The problem is rootCmd.Execute() does not take any string args, and possibly the subcommands are also not thread safe.

@jharshman
Copy link
Collaborator

jharshman commented Jul 15, 2019

In short, if you are creating cobra.Command structures "on the fly" as messages come in, and you want this to be thread safe; I don't believe the specific "thread safe" feature you are looking for exists.

@umarcor
Copy link
Contributor

umarcor commented Jul 15, 2019

@mustafaakin, IMHO, you should try to provide a more detailed example that explains why you need to create cobra.Command structures on the fly. AFAIK there is no limitation at all to "execute same command concurrently". This is possible either by calling the same binary multiple times, with different CLI arg values; or starting a server that listens at a port/socket (I believe this is what @jharshman suggested with "listen for slack message events and then do something for that event").

If you feel that we are not answering your question, please be specific. E.g., can you provide a couple of examples of the messages that you expect to receive from Slack and the functions that you expect cobra to generate for you? A MWE would be great.

@jharshman
Copy link
Collaborator

Yes there is too much obscurity in what is attempting to be accomplished here. I find myself instead guessing. @mustafaakin can you please provide a more detailed example or perhaps a link to the code itself?

@lieut-data
Copy link

I'm actually proposing doing the same thing (except for Mattermost, not Slack) in this help wanted ticket: mattermost/mattermost#11843.

We currently have a slash command model that allows users to type a message like:

/plugin do something --cool

but plugin authors have to process this sequence of instructions manually. In the ticket, I propose an idea to proxy the do something --cool through Cobra so that the plugin author can use all the cool stuff in this package.

However, Cobra's interface currently seems to assume a single-threaded model. If my plugin was processing requests for two slash commands at the same time, I couldn't safely call cobra's SetArgs and expect Execute to work reliably.

Ideally, I'd have something like Execute(args), though I'm sure there's probably more to it. It might suffice just to synchronize access to the cobra command, effectively serializing all requests, but like @mustafaakin, I'm curious if any thought to this topic has been given within this package.

@umarcor
Copy link
Contributor

umarcor commented Aug 9, 2019

@lieut-data, if I didn't get it wrong, it is possible to prototype what you want by building two binaries, as suggested above. One would be the cobra-pased consumer, and the other one your interface/producer. That will let you do your thing, independently of when/how this issue is solved.

When the prototype is ready, the next step would be to expose the entrypoint to the consumer, import it as a library of the producer and replace the call to the external program with a call to the entrypoint. This will provide a solid starting point for anyone willing to investigate concurrency issues.

It might suffice just to synchronize access to the cobra command, effectively serializing all requests,

I believe that the underlying issue is not that cobra is single-threaded, but thought for a single execution. I'd say that the point here is that no executable CLI app is meant to receive multiple sets of args. For each call to the binary, the OS creates an 'isolated' context where a single set of args exists. Hence, I think that most of the underlying logic in cobra and/or pflag is valid; but someone needs to check through all the code where it is assumed the the set of flags is unique.

As a result, I don't know if serializing the requests will work. On the one hand, there is no cleanup mechanism, so some global variables might be altered when the second or subsequent messages arrive. On the other hand, some users rely on os.Args being unique; hence, developers of commands/subcommands should be instructed.

FTR, pflag is a library to process CLI arguments and cobra is a library to compose trees of commands/subcommands which are prepended to CLI arguments. It should be possible to adapt both of them so that 'CLI' is removed from the description.

This is probably related to https://golang.org/pkg/context/ and #893.

@lieut-data
Copy link

Thanks for the reply, @umarcor, and sorry for my own delay in responding.

... if I didn't get it wrong, it is possible to prototype what you want by building two binaries, as suggested above. One would be the cobra-pased consumer, and the other one your interface/producer. That will let you do your thing, independently of when/how this issue is solved.

The challenge here is that I really just want to use cobra as a router to process "CLI-like args". The desired command implementations are necessarily colocated with the code processing the slash command.

I believe that the underlying issue is not that cobra is single-threaded, but thought for a single execution. I'd say that the point here is that no executable CLI app is meant to receive multiple sets of args.

You're absolutely right, and it makes sense for cobra to work under this assumption. Thanks for confirming that this just isn't in the scope of the package right now.

I'd personally love to see Cobra generalized not to assume single execution -- #893 would be one such step towards that effort. Is there an interest in pulling towards this direction? If that PR was merged, it /might/ not be that much more effort for community members like myself to help push this across the line.

@umarcor
Copy link
Contributor

umarcor commented Dec 5, 2019

I'd personally love to see Cobra generalized not to assume single execution -- #893 would be one such step towards that effort. Is there an interest in pulling towards this direction? If that PR was merged, it /might/ not be that much more effort for community members like myself to help push this across the line.

As said, I think that it would be interesting:

FTR, pflag is a library to process CLI arguments and cobra is a library to compose trees of commands/subcommands which are prepended to CLI arguments. It should be possible to adapt both of them so that 'CLI' is removed from the description.

So, getting the environment/context from the os, and the args from stdin, would be just a specific (the default) usage.

Unfortunately, this would be a quite disruptive change. See #959. I wish we had some v2 to work on while this v1 is kept mostly untouched.

@github-actions
Copy link

github-actions bot commented Apr 5, 2020

This issue is being marked as stale due to a long period of inactivity

@hammypants
Copy link

I'm supremely interested in this concept existing in a much better form in cobra. Is there a v2 plan anywhere, however young? Some rudimentary searches of the issues list did not turn up anything, and I don't see a branch. Even some small headway into this, like not storing contexts (per the context documentation), would help a lot.

@johnSchnake johnSchnake added kind/feature A feature request for cobra; new or enhanced behavior area/cobra-command Core `cobra.Command` implementations lifecycle/needs-proposal For large features/bugs that need a proposal and community buy in labels Aug 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cobra-command Core `cobra.Command` implementations kind/feature A feature request for cobra; new or enhanced behavior lifecycle/needs-proposal For large features/bugs that need a proposal and community buy in
Projects
None yet
Development

No branches or pull requests

6 participants