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

Make command timeout configurable #21

Closed
gootik opened this issue Sep 20, 2020 · 7 comments · Fixed by #23
Closed

Make command timeout configurable #21

gootik opened this issue Sep 20, 2020 · 7 comments · Fixed by #23

Comments

@gootik
Copy link
Owner

gootik commented Sep 20, 2020

No description provided.

@paulo-ferraz-oliveira
Copy link
Contributor

What about syntax?

How does this look?

{"make test", [{timeout, 5000}]} % in case you want more options in the future

or simpler still

{"make test", 5000} % you could still add more options with a new syntax and "deprecate this one"

@gootik
Copy link
Owner Author

gootik commented Sep 24, 2020

looks good but lets support default configs too:

{commands, [
 {a, "make test", [configs...]},
 {b, "make run"}
]}.

@paulo-ferraz-oliveira
Copy link
Contributor

You mean not using an option list? Sure, I was not mentioning otherwise (I was giving examples for the "extra" interface - since I don't want to break existing implementations/usage.

@gootik
Copy link
Owner Author

gootik commented Sep 24, 2020

sorry I meant we should support both of the cases in my example. where [configs..] is your suggestion. So my revised example of things that should be supported:

{commands, [
 {a, "make test", [
    {timeout, 5000}, 
    {ignore_output, true} % just an example, not suggesting we should do this
 ]},
 {b, "make run"}
]}.

@paulo-ferraz-oliveira
Copy link
Contributor

Communication 😄

When I wrote You mean not using an option list? I should've written Do you mean "not using an option list at every declaration" (and using a default list of options, otherwise) when you state "lets support default configs too"?

I got it, though. I wouldn't force a list if I didn't have too, anyway.

@gootik
Copy link
Owner Author

gootik commented Sep 26, 2020

Hahaha. Sounds good

@paulo-ferraz-oliveira
Copy link
Contributor

I've started fiddling with the implementation, for this issue, but have some questions.

What does the notion of "timeout" mean here, exactly? I don't think we can easily "stop command execution" on the OS once it's been started by os:cmd. I maybe missing something, though.

It would be OK, for example, if you "chain" commands.

e.g.

{slow_cmd, [
    {"find / -name .gitignore ", [{timeout, 2000}]},
    "ls"
]}.

Here, timeout could prevent the execution of ls.

And another thing, would you still consider the command result for output even if it arrived late? Or just ignore it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants