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

do not redispatch systematically when we're in backend mode #684

Closed
wants to merge 1 commit into from
Closed

do not redispatch systematically when we're in backend mode #684

wants to merge 1 commit into from

Conversation

anarcat
Copy link
Contributor

@anarcat anarcat commented Jun 20, 2014

since the fix for #555, commands are needlessly redispatched by drush when called with --backend. previous to commit 24addcd, this didn't take effect because the DRUSH_BACKEND context wasn't initialised properly when we were loading aliases. but now that we set that context earlier, the check takes effect, and things get redispatched like crazy.

this seems to happen before Aegir/provision has time to set its own stuff properly (i am guessing: load the good stuff from the alias) so the redispatch fails to load with the proper alias. more details on that are on the aegir issue in https://drupal.org/node/2281983.

this was originally introduced in 510735a to fix #1058874, but the commitlog makes no reference to backend at all, and merely references an obscure php-options variable. the issue mentions something about interactivity in backend mode, but that doesn't matter since it wasn't actually triggered before anyways.

we have yet to run unit tests on this, but with this patch, aegir can install again on drush master.

this didn't take effect before
24addcd because backend wasn't
initialised properly when we were loading aliases back then

fixes #2281983
@ergonlogic
Copy link
Member

ergonlogic commented Jul 8, 2014

In drush_preflight_command_dispatch(), if --remote-host is set (ref.), it'll trigger redispatching (ref.).

e4c652e moves the setting of --backend earlier in the Drush bootstrap; from within _drush_bootstrap_output_prepare() to _drush_bootstrap_global_options(). This puts it before drush_preflight_command_dispatch(), thus triggering the re-dispatch.

This pull request simply removes --backend from being a sufficient condition to set --remote-host. This wasn't an issue previously, since --backend never got set early enough to trigger it.

@greg-1-anderson
Copy link
Member

greg-1-anderson commented Jul 8, 2014

After some head-scratching, I think that the above-mentioned code was intended to be:

if (array_key_exists('php-options', $site_alias_settings) && !drush_get_context('DRUSH_BACKEND', FALSE))

That commit was a long time ago, so it's hard to remember what we were thinking. However, I can't think of any good reason why --backend should force a dispatch, but I can think of a good reason to avoid redispatch with --backend, so I have to presume that the code as committed was a typo, and it wasn't caught only because we didn't realize that --backend was not processed until later.

I'm fine with this PR as written, but I think it'd be even better to add in the 'and NOT backend' test as well.

ergonlogic added a commit to PraxisLabs/drush that referenced this pull request Jul 10, 2014
@ergonlogic
Copy link
Member

ergonlogic commented Jul 10, 2014

I added a commit (to my fork) as per Greg's comment: PraxisLabs@72e167c

@ergonlogic
Copy link
Member

ergonlogic commented Jul 14, 2014

This is RTBC according to Moshe in IRC. Is there a way to label it, or other workflow to mark this?

@weitzman weitzman closed this in 2ab875e Jul 16, 2014
@anarcat anarcat deleted the bug/backend-redispatch branch Jul 18, 2014
@anarcat anarcat restored the bug/backend-redispatch branch Jul 18, 2014
@anarcat anarcat deleted the bug/backend-redispatch branch Dec 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants