Set infinite timeout for local commands

[mbrodala]

Q	A
Bug fix?	Yes
New feature?	No
BC breaks?	No
Deprecations?	No
Fixed tickets	N/A

Very long running commands (e.g. DB import) can exceed even 5 minutes (see #955) thus switch to an infinite timeout for local commands.

[antonmedv]

Why not runLocally('...', INF)?

[mbrodala]

@Elfet Yes, for runLocally() this is possible but not with run() on a localServer().

[antonmedv]

not with run() on a localServer().
I think we need to fix API and allow to set timeout for this too.

run('...', INF);

[mbrodala]

I'd be fine with that too. Notice that Symfony expects null for infinite timeouts, not INF.

[pluseg]

@Elfet What do you think about null value for infinite timeouts? It blocks the PR.

[antonmedv]

Null is okay, but i like INF for verbosity. Also i'm thinking about changing run api in v5 to something like this: run('...', $options).
To becable pass timeouts, pty and other option for current run.

[mbrodala]

@Elfet Sounds good. How to proceed on this PR? I can rebase to fix the conflicts if you think it makes sense to have this added.

A little more background: with custom tasks you can of course manually pass null to enable infinite timeout, however this is not possible with 3rd party tasks/recipes.

[antonmedv]

I will manually merge this pr later.

however this is not possible with 3rd party tasks/recipes.

This is interesting point, we need found good API for it. What are problems to solve we want?

[mbrodala]

@Elfet That won't be easy to fix. We can reconfigure tasks by now but not the function calls (e.g. runLocally()) within those tasks.

[antonmedv]

Done in #1092

run('...', ['timeout' => null]);
runLocally('...', ['timeout' => null]);

[antonmedv]

Also i was thinking about renaming runLocally to exec.

[mbrodala]

@Elfet The change you mentioned does not solve the issue mentioned here: I'd still need to copy the body of all tasks from 3rd party recipes to set the timeout option. Even i these recipes allow me to customize their command options, this can already be achieved with the current API, see add('rsync', ['timeout => N]);

Thus please reconsider this PR.

[staabm]

Also i was thinking about renaming runLocally to exec.

I like run and runLocally as they cleary communicate their purpose.
exec and run dont make a statement whether they run remotely or locally... this would harm readability

[antonmedv]

@mbrodala i see, maybe it's should be 3rd party issue? I like simplicity of options in run function. For example, this is how i pass tty to git clone command: ace2889#diff-b4ebd1b9bf76c38d2568949ec0c70c51

Of cause we can add some global param, but i don't like it for some reason.

[mbrodala]

You are probably right that the 3rd party recipes should offer options to customize the timeout.