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

Reconfigure tasks #954

Closed
mbrodala opened this issue Jan 13, 2017 · 18 comments
Closed

Reconfigure tasks #954

mbrodala opened this issue Jan 13, 2017 · 18 comments
Labels

Comments

@mbrodala
Copy link
Contributor

Q A
Issue Type Feature Request
Deployer Version 4.0.2
Local Machine OS N/A
Remote Machine OS N/A

Description

It would be useful if there was an API to reconfigure existing tasks. Right now if one wants to e.g. limit execution of a task from a recipe to a limited set of servers, one has to do something like this:

desc('Limit foo task to selected servers');
task('config:foo:bar', function() {
  $deployer = Deployer::get();
  $task = $deployer->tasks->get('foo:bar');
  $task->onlyOn([
    'server1',
    'server2',
  ]);
});
before('foo:bar', 'config:foo:bar');

I could imagine an API like this instead:

task('foo:bar')->onlyOn([
  'server1',
  'server2',
]);

(Same goes for all other methods of the Task class.)

This would mean that the $body of task() would become optional. If called with only the task name, the Task object would be returned if present.

What do you think?

@antonmedv
Copy link
Member

I think reconfiguring tasks on runtime is bad idea. Why you need it for?

@mbrodala
Copy link
Contributor Author

I gave an example above: a task bundled with a recipe should only be executed for a selected list of servers (or stages). To be specific its a task which should clear the PHP opcode cache and that doesn't work on all of our servers. Thus we limit the task to the servers where it works. Yes, the original issue is the broken servers but sometimes we don't have a choice and thus try to work with it as good as possible.

And yes, I understand that reconfiguring tasks at runtime can have bad side effects.

Do you have another idea how we could solve our (and similar) issues?

@antonmedv
Copy link
Member

I think API like this task('foo:bar') is okay. Need to think a little bit more about corner cases.

Also did you try this?

task('foo:bar:part', ['foo:bar'])->onlyOn([
  'server1',
  'server2',
]);

Using group tasks for this?

@mbrodala
Copy link
Contributor Author

Your suggestion sounds useful and would at least reduce the visual overhead, I'll try that.

@antonmedv
Copy link
Member

I will add this solution to docs later.

@mbrodala
Copy link
Contributor Author

mbrodala commented Feb 10, 2017

@Elfet I just noticed that this is not possible actually since group tasks cannot be configured like this.

Do you think it would be possible to allow this for group tasks? Otherwise I'd have to stick to my original attempt, unless task reconfiguration is added.

@antonmedv
Copy link
Member

@mbrodala can you describe when you want to configure tasks? At what point and based on what data?

@mbrodala
Copy link
Contributor Author

@Elfet I think I made this clear by now. ;-) We have some tasks (e.g. PHP Opcache reset) which are included from recipes but should only run on a few selected servers since e.g. reset does not work on the other servers.

@antonmedv
Copy link
Member

Ok, so this kind of API will be usable here:

task('deploy:opcache')->onlyOn([
  'server1',
  'server2',
]);

@mbrodala
Copy link
Contributor Author

Yeah, that's my suggestion. I can add a PR if you think this could go in.

@antonmedv
Copy link
Member

antonmedv commented Feb 10, 2017

Yes, i think les't give it a try)

@pluseg
Copy link
Contributor

pluseg commented Feb 17, 2017

@Elfet @mbrodala guys, I guess the issue can be closed, doesn't it?

@antonmedv
Copy link
Member

True

@dbalabka
Copy link
Contributor

dbalabka commented Feb 27, 2017

task('deploy:opcache')->onlyOn([
  'server1',
  'server2',
]);

got following error:

PHP Warning:  Missing argument 2 for Deployer\task(), called in 
$ php ./deployer.phar --version
Deployer 4.2.1

@mbrodala
Copy link
Contributor Author

@torinaki There is no release of Deployer which includes this change yet.

@dbalabka
Copy link
Contributor

@mbrodala but offical documentation already contains this changes:
image
https://deployer.org/docs/tasks

@mbrodala
Copy link
Contributor Author

Yeah, because there is only one branch (master) in that repository. @Elfet Maybe this should be changed?

@antonmedv
Copy link
Member

Yes, i really wan't to do something with this, maybe have two branches, published and master.

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

No branches or pull requests

4 participants