Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
DrupalSqlBase::getSystemData() has a design quirk that it eats exceptions and just treats the failures as an empty system data array. This works fine in practice but its also the core of the logic in DrupalSqlBase::checkRequirements() where this is a usability issues.
For example, a connection string error shows the following for the d6_filter_format migration:
[ok] Running d6_filter_format
[error] Migration d6_filter_format did not meet the requirements. The module filter is not enabled in the source site. source_module: filter.
It would be a lot more useful for the requirements check to check the database first and see something like:
[error] SQLSTATE[HY000] [1045] Access denied for user 'migration_user'@'localhost' (using password: YES)
Comment | File | Size | Author |
---|---|---|---|
#8 | 3008720-7.patch | 2.14 KB | dhirendra.mishra |
#3 | 3008720-3.patch | 2.28 KB | quietone |
#3 | fail-test.patch | 1.41 KB | quietone |
#2 | better_DrupalSqlBase_requirements_check.patch | 892 bytes | neclimdul |
Comments
Comment #2
neclimdulGood news, SqlBase already does this check, we just run it in the wrong place to ever catch it.
Comment #3
quietone CreditAttribution: quietone as a volunteer commentedI thought a test might be useful and tried to make one.
Comment #5
neclimdulThat's awesome! I was kinda dreading what a test might look like but that's not bad.
Do we need to catch a re-throw? seems like if we just let it go it'll work as expected already.
Comment #6
heddnYeah, I think removing the try catch would be better.
Comment #7
dhirendra.mishra CreditAttribution: dhirendra.mishra at Srijan | A Material+ Company commentedworkinmg on it.
Comment #8
dhirendra.mishra CreditAttribution: dhirendra.mishra at Srijan | A Material+ Company commentedRemoved the try catch as suggested in previous comment. Please review my patch.
Comment #9
heddnAssuming this comes back green, onward!
Comment #10
neclimdulThis might be the fastest to RTBC code related issue I've ever been on. Thanks! Awesome work and +1 RTBC.
Comment #11
alexpottCommitted and pushed 5123f7568d to 8.7.x and b6f8992600 to 8.6.x. Thanks!