Add engine for the sql connection credential commands. #483
Conversation
|
This looks OK. I'd rather use classes now instead of engines like /lib/Role, for example. But a Drush maintainer can do that. What I'm curious about is if this patch works for sql-sync (and if not, does sql-sync work without the patch). Hopefully someone can test that soon. |
| function _drush_get_db_settings($database, $target) { | ||
| if (drush_bootstrap_max(DRUSH_BOOTSTRAP_DRUPAL_CONFIGURATION)) { | ||
| // We don't use DB API here `sql-sync` would have to messily addConnection. | ||
| if ($info = Database::getAllConnectionInfo()) { |
There was a problem hiding this comment.
Does this really work? The whole premise of DRUSH_BOOTSTRAP_DRUPAL_CONFIGURATION is that Drush has included settings.php but no other part of Drupal. Therefore, I'm surprised that an autoloader would be available to handle Database class instantiation. Has anyone tested site-install with this PR? Another good thing to test is to configure settings.php and point it at a random DB (not Drupal). sql-query should still work. I don't think it will with this patch. Aegir relies on this AFAIK. sql-sync as well.
|
The autoloader has to be initialized by that point for Settings() etc.. atleast that is the case in _drupal_bootstrap_configuration(). Which I guess drush calls at come point in the form of drupal_bootsrap(DRUPAL_BOOTSTRAP_CONFIGURATION)? |
|
#504 has landed so this should be simpler now. |
Drupal 8 has removed the $databases global https://drupal.org/node/2176621. Drush relies on this.