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

Multisite installs do not load drush.yml #3968

Closed
greg-1-anderson opened this issue Mar 4, 2019 · 15 comments · Fixed by #4345
Closed

Multisite installs do not load drush.yml #3968

greg-1-anderson opened this issue Mar 4, 2019 · 15 comments · Fixed by #4345

Comments

@greg-1-anderson
Copy link
Member

Describe the bug
drush.yml is not loaded for multisite installs.

To Reproduce
Put a drush.yml in web/sites/default/drush.yml

Expected behavior
The path to that drush.yml file should appear in drush status

Actual behavior
Nothing at all.

Workaround
Unknown; maybe set uri in settings.php. Drush.yml might be used for other things though.

System Configuration

Q A
Drush version? 9.6.0-rc3

Additional information
Drush is attempting to add the configuration in ConfigLocator, but something is amis.

Fixing this was previously attempted and rejected in #3278.

@greg-1-anderson
Copy link
Member Author

Let's give some folks a chance to fix this per what we're doing in #3966 before we close this again. I think that if we looked up uri in configuration, just as #3966 is doing with preflight args, and inject overrides into the site alias, then it could work. We'd have to be careful to only check site-specific configuration, as we would not want a uri in a global location to override a uri in a site alias. We have the ability to do this in our ConfigOverlay. ConfigLocator might need to cooperate.

@greg-1-anderson
Copy link
Member Author

greg-1-anderson commented Mar 4, 2019

Ah ha, we have: https://github.com/drush-ops/drush/blob/9.6.0-rc3/src/Config/ConfigLocator.php#L108

n.b. "not implemented yet (multisite)"

@greg-1-anderson
Copy link
Member Author

Good news! addSitewideConfig is called before findSite, so it should be possible to fix this as I proposed.

@greg-1-anderson
Copy link
Member Author

There's one interesting possible side-effect here:

  • Drupal site / uri implicitly selected (e.g. via cwd to sites/sitename)
  • sites/sitename/drush/drush.yml sets a more specific uri
  • The SITE bootstrap (Drupal) selects a different multisite based on the more specific uri

This would just be a user configuration bug, though; we could potentially detect it and throw an error if it was a concern.

@greg-1-anderson
Copy link
Member Author

Any proposed solution here should also consider the impact of using sites.php.

@greg-1-anderson
Copy link
Member Author

It might be better to document that multisite installations should use sites.php to set the uri rather than doing so in a site-local drush.yml. Anecdotally, that is reported to work.

Users might want to have a site-local drush.yml for some other purpose, although at the moment a plausible use-case does not come to mind. Perhaps we can postpone action here until that is clear.

I'd still like to finish #3966 though.

@weitzman
Copy link
Member

weitzman commented Mar 5, 2019

FYI sites.php maps one incoming URI to another, more "normalized" URI. Its just a mapping array. sites.php cannot set a URI from nothing. See https://github.com/drupal/drupal/blob/8.6.x/sites/example.sites.php

@greg-1-anderson
Copy link
Member Author

I was going to say that I don't have any multisites to confirm, but the SUT is a multisite. I thought I would try to use sites.php to correct the URI for the staging multidev:

Default answer:

$ ./drush @sut.stage st --field=uri 
http://stage

Upon trying it, I discovered that sites.php maps in the other direction. It can go from https://stage.example.com to the stage directory, but it cannot produce https://stage.example.com if the URI is passed in as stage.

So, in conclusion, it seems that sites.php is orthogonal to this feature request. I suppose my concern above might still be an issue, but I didn't try to reproduce.

@greg-1-anderson
Copy link
Member Author

This is related to #3966, but I'd still like to consider fixing it. There may be limitations as described above, but I think it might be worth doing. We should give it a shot, anyway.

I might not have time to try it until after DrupalCon Seattle.

@greg-1-anderson
Copy link
Member Author

I will also mention that regardless of whether we enable setting the uri from a mulitsite drush.yml file, I think that multisites should still get their own drush.yml files in case they need them for other purposes.

@danepowell
Copy link
Contributor

Not all infrastructures support the use of sites.php, and I'm told that there are other types of configuration that would be nice to set via site-specific drush.yml or local.drush.yml files. So I think this would still be nice to have.

@ndobromirov
Copy link

ndobromirov commented Aug 1, 2019

Any progress on this one?
Any plans to have a fix on it?
Customizing drush settings per site will greatly help in relation to #3594 where the cache-directory can be safely generated based on site.

It's inconvenient to have the fix with the variable, as it will end up being executed / assumed in SO many places...

Can we increase the priority of the ticket. It's critical for stable management if multi-sites.

@weitzman weitzman removed the pri-low label Aug 1, 2019
@weitzman
Copy link
Member

weitzman commented Aug 1, 2019

If you have environment variables per site, then you can set Drush config that way. Or you can specify config files with --include ... I have no alternative suggestion. I increased the priority though I doubt that will actually accomplish anything. Folks who still use multisite (I dont recommend it) are welcome to submit a PR.

@greg-1-anderson
Copy link
Member Author

Post DrupalCon Seattle, I've been pretty busy with the Composer In Core Initiative & c., so I haven't really had time to look at things like this. I don't use multisite myself and don't recommend it, but would still be interested in seeing this feature implemented. I would also encourage folks who use multisite to submit a PR.

@richardbporter
Copy link
Contributor

I gave this a shot in #4345 and referenced any issues I thought were related. Any feedback would be appreciated.

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.

5 participants