while working on #1597696: Consider whether HttpCache offers any significant benefit over the existing page cache, i noticed that we can't call config() before _drupal_bootstrap_database(), because if we do, the global $databases array hasn't been munged with the test prefix.

this means that all calls to the db on the test side hit the parent drupal, and we get ~14k fails.

so this patch moves the global $databases munging to _drupal_bootstrap_configuration(), making it safe to call config() before _drupal_bootstap_database().

Files: 
CommentFileSizeAuthor
kill-drupal-bootstrap-db.patch2.1 KBbeejeebus
PASSED: [[SimpleTest]]: [MySQL] 40,376 pass(es). View

Comments

beejeebus’s picture

Status: Active » Needs review

ahem, set status so bot sees this.

beejeebus’s picture

just wanted to note chx's work over at #1764474: Make Cache interface and backends use the DIC, which also touches global $databases and bootstrap stuff.

sun’s picture

Title: move test prefix $databases munging earlier in the bootstrap » Move test prefix $databases munging earlier in the bootstrap
Issue tags: +Framework Initiative, +WSCCI, +Configuration system, +Testing system

Thanks! :) Adding a full range of tags there... ;)

Oy. Unfortunate diff context there. "Looks good" but I'll have to apply it and diff visually to verify the changes.

sun’s picture

Category: bug » task
Priority: Major » Normal

This is important, but let's not hold up other patches that are ready. :)

sun’s picture

Status: Needs review » Reviewed & tested by the community

So I applied this patch locally to review the larger diff context some more, and this is a lovely change.

That said, the remainder of DRUPAL_BOOTSTRAP_DATABASE is pretty silly:

/**
 * Initializes the database system by loading database.inc.
 */
function _drupal_bootstrap_database() {
  // Redirect the user to the installation script if Drupal has not been
  // installed yet (i.e., if no $databases array has been defined in the
  // settings.php file) and we are not already installing.
  if (empty($GLOBALS['databases']) && !drupal_installation_attempted()) {
    include_once DRUPAL_ROOT . '/core/includes/install.inc';
    install_goto('core/install.php');
  }
  // Initialize the database system. Note that the connection
  // won't be initialized until it is actually requested.
  require_once DRUPAL_ROOT . '/core/includes/database.inc';
}

At least this reads and feels very silly in days of PSR-0 autoloading... ;)

However, potentially removing that artifact and improving the bootstrap phases is a separate issue.

Let's do this! :)

(This patch is a blocker for #1597696: Consider whether HttpCache offers any significant benefit over the existing page cache)

Crell’s picture

I'm OK with this, too. The only reason the database bootstrap would still exist is for the db_* functions. I'm sorely tempted to just move those to bootstrap.inc, make them a Drupal-only wapper, and blast this bootstrap step completely. A later patch, though. :-)

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Seems simple enough.

Committed and pushed to 8.x. Thanks!

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