Most of the SQL commands require a minimum bootstrap level of DRUSH_BOOTSTRAP_DATABASE because they need to operate on a database (of course). Since one can supply a database via --db-url option, I think we should reduce the DRUSH_BOOTSTRAP_DRUSH. Any objection? I refer to sql-query, sql-drop, and others.

Comments

greg.1.anderson’s picture

Yes, I agree in concept, but I think that in principal the current code might need to be DRUSH_BOOTSTRAP_DRUPAL, because we do not know what the $db_url is until then. It would be possible to recode the bootstrap process so that we decide what site we are going to target in DRUSH_BOOTSTRAP_DRUSH, but hold off on actually bootstrapping Drupal until the next phase. This would improve some other code in the bootstrap process, but it would not help us get $db_url, because we currently get that by including settings.php, which pollutes our global environment. I don't suppose there's any way to include a php file in another context? I suppose we could 'exec' <?php include 'settings.php'; print $db_url; if we really wanted to.

In conclusion, I think we could pull the requirement back to DRUSH_BOOTSTRAP_DRUPAL without changing the bootstrap code. Didn't try it, though.

moshe weitzman’s picture

Assigned: Unassigned » moshe weitzman

Right. I think I'll reduce the 'bootstrap' level and in a validate hook:

if (!drupal_get_option('db-url)) {
  drush_bootstrap_max(DRUSH_BOOTSTRAP_DATABASE);
}
greg.1.anderson’s picture

I think maybe DRUSH_BOOTSTRAP_CONFIGURATION is enough to get $db_url, but yes, the above is good.

moshe weitzman’s picture

Status: Active » Patch (to be ported)

Committed. Lets backport for 4.6

glennpratt’s picture

Version: » All-versions-4.x-dev
moshe weitzman’s picture

I'm pretty undecided about the benefit/risk for backporting that commit. Anyone have an opinion?

greg.1.anderson’s picture

As far as risk is concerned, I think this is safe to backport. As for benefit vs. effort, I find this patch useful in Drush 5, but I don't personally think it is too important to backport. Certainly could be done if anyone felt like it, though.

msonnabaum’s picture

Status: Patch (to be ported) » Fixed

Backported this, which needed a few more to get tests passing again.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.