drush_valid_db_credentials() is used to validate the bootstrap to DRUSH_BOOTSTRAP_DRUPAL_DATABASE. If it does return TRUE the bootstrap can happen. There's a problem in some circumstances where it return TRUE and it will not prevent further errors.
drush_valid_db_credentials() assumes yes and return TRUE:
* if PDO is not available
* if PDO is available but mysql driver is not available
if it does return TRUE the bootstrap validation success and so does the bootstrap itself, that is just printing a not-always valid message: "Successfully connected to the Drupal database.".
I've made some tests with drush cache clear reproducing #567062: command "drush status" returning HTML-output and error "Drush command could not be completed." and those are the facts I've seen:
- If no PDO-mysql and no php-mysql this is the drush/drupal output:
PDO support not available. Could not pre-validate database credentials. Assuming success Drush bootstrap phase : _drush_bootstrap_drupal_database() Successfully connected to the Drupal database. {drupal dumps a maintenance page because it is not able to connect to the database}. - If PDO-mysql but no php-mysql driver: drush is able to connect and verify db credentials but drupal is not able to connect to the database. Same messages as above but the first one, as PDO-mysql is available.
- If no PDO available and php-mysql driver available: drush will not be able to verify credentials (it will "assume yes") and drupal is able to connect and do the work.
PDO support not available. Could not pre-validate database credentials. Assuming success Drush bootstrap phase : _drush_bootstrap_drupal_database() Successfully connected to the Drupal database.
Drupal 7 will come with PDO support so I figure out we will get rid of this asymmetry but in drupal 5,6 using PDO in drush and php-mysql will need to handle special cases and extra checks. For sure we must check php-mysql is available in php-cli. We can assume yes if PDO is not available but we also need to check that php-mysql is available and can't assume yes on that.
I wonder what's the need for PDO currently.. if it is not used anywhere else in drush (nor aegir) but to validate the database bootstrap phase.
Do we need to simply add a extra check for php-mysql if drupal version in (5,6) or refactor that function to check credentials by using php-mysql or PDO based on the drupal version?
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | drush-571352.patch | 1.47 KB | jonhattan |
| #14 | drush-571352.patch | 1.47 KB | jonhattan |
| #8 | drush-571352.patch | 6.14 KB | jonhattan |
| #4 | drush-571352.patch | 6.18 KB | jonhattan |
Comments
Comment #1
moshe weitzman commentedComment #2
moshe weitzman commentedI'm not married to PDO for this. We can check for php-mysql. Patches wanted.
Comment #3
jonhattanComment #4
jonhattanHere's a rewrite. It uses mysql / pgsql or PDO based on the drupal version you'r bootstrapping. It doesn't implement a separate case for mysqli but perhaps it is oportune to just avoid other edge case.
It still needs some polish in error messages and also testing against postgres. To see the error messages #962754: debug messages not available until DRUSH_BOOTSTRAP_DRUSH overcome is needed.
Comment #5
moshe weitzman commentedAt minumum, we need to test this on D6/D6/D7 mysql. Ideally we would do same for pgsql. Can anyone confirm that these cases work? I'd like to commit this soon.
Comment #6
greg.1.anderson commentedI'll do the postgres testing. I'll need to install ubuntu 10.10 to do d7 + postgres, as d7 requires a certain minimum version of php to work. Will even do d5 + postgres.
Comment #7
greg.1.anderson commentedThis patch works as expected under D6 + pgsql.
Comment #8
jonhattanPrevious patch didn't work for D7.
I doubt if using
drush_bootstrap_error('ERROR_ID', $message)ordrush_log($message, 'bootstrap');. Adrian did use drush_log() and this patch does so. Previous one did use drush_bootstrap_error().Advantage of using drush_log() is that errors are printed as they happen but it needs --debug. Advantage of drush_bootstrap_error() is that errors are always printed if the command doesn't hit the needed bootstrap level but they're printed unordered, at the end. Anyway, teaching users to report bugs with --debug is the way to go.
Comment #9
greg.1.anderson commented#8 works as it should with D5 and D6 + pgsql.
Comment #10
moshe weitzman commentedAwesome work.
Comment #11
jonhattanCommited.
Comment #13
sunCritical problem: mysql_connect() may perfectly be not available when $type is mysqli.
One of my environments uses mysqli, so PHP's mysql extension has been purposively left out, since only mysqli is needed, and I'm getting:
Powered by Dreditor.
Comment #14
jonhattanImplemented. It works here. Marking as needs review just in case I missed something.
Comment #15
jonhattanIndeed I missed the db name.
Comment #16
sunThanks, that resolves the fatal error.
Comment #17
jonhattancommitted.
Comment #19
kotnik commentedThis issue: #1112984: Mysqldump is the same for mysql i mysqli drivers is related as well.