Allow task servers to override story servers

[amsoell]

Happy to take a stab at a PR here but I wanted to confirm I wasn't missing something. Consider the case where you want to update multiple staging servers, and as part of it download a production db snapshot and restore it to staging:

@servers(['staging-servers' => ['alpha.staging.example.com', 'beta.staging.example.com'], 'staging-primary' => 'alpha.staging.example.com'])

@story('stage', [ 'on' => 'staging-servers' ])
    down
    pull
    dependencies
    restoredb
    migrate
    up
@endstory

Most of these task, of course, we want to run on all servers. However, we have a "restoredb" task that grabs the latest production database backup and restores it to staging, so we can work with the latest data. We obviously only want this to run one time.

I would expect that I could override the on setting on the restoredb task specifically:

@task('restoredb', [ 'on' => 'staging-primary' ])
    ...
@endtask

But that doesn't seem to work. So first, is this expected? Second, if it is expected, is there a different way we ought go about this, and is there a downside to changing behavior so it does work this way?

[driesvints]

I think you've misread the docs a bit. Can you try the example from below?

@servers(['staging1' => 'alpha.staging.example.com', 'staging2' => 'beta.staging.example.com'])

@story('stage', [ 'on' => ['staging1', 'staging2'])
    down
    pull
    dependencies
    restoredb
    migrate
    up
@endstory

@task('restoredb', [ 'on' => 'staging1'])
    ...
@endtask

[amsoell]

@driesvints Here's an easier example, which I think fits your format:

@servers(['amsoell' => 'amsoell.com', 'plexz' => 'asteria.feralhosting.com'])

@story('deploy', ['on' => ['amsoell', 'plexz']])
    touch1
    touch2
@endstory

@task('touch1')
    touch one
@endtask

@task('touch2', ['on' => 'plexz'])
    touch two
@endtask

When I run this and check the amsoell.com server I see both files, which should not be the case.

[driesvints]

Okay, I went through the internals a bit and managed to figure out what's wrong. Because of the line below, the macro/story options always overwrite the task options. I'm not sure what the correct expectation would be. Either the current behavior or the other way around.

envoy/src/TaskContainer.php

Line 331 in 0250f33

$options = array_merge($this->getTaskOptions($task), $macroOptions);

I any case if we were to flip the params in the array_merge I believe it would use the behavior you described above. However, this would be a severe breaking change and would have to target the next major version at least. At the moment I'm not sure if it's wise to change it. We'll need some more opinions on this first.

I'll mark this as an enhancement since it's not actually a bug.

[amsoell]

Thanks for checking into that for me. I can’t imagine a scenario where someone would want the story servers to override task, but I suppose that doesn’t mean they don’t exist.

Short of changing this, if I put the work on not adding a single_server argument similar to the parallel argument, would that be viewed as a good, merge-worthy feature?

[driesvints]

@amsoell I don't think so. It'll be a band-aid for the real problem. I don't think it'd get merged.

[driesvints]

I've sent in a PR for this here: #157

[driesvints]

We've reverted this as this started to break people's apps. We won't be reconsidering this sorry.

#186

[Tofandel]

Well actually there could have been a way to fix the issue without reverting the change, the problem seems to be that the task has a default 'on' when not specified which targets all server, and so the story 'on' gets overridden even though there is no 'on', this could have been caught with some unit test, the default 'on' should only be applied after merge

But ideally we could just look for an option in story called opts_override and if set to false change the inheritance behaviour which wouldn't be a breaking change

Because right now file inheritance is unusable because of this