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.
With migrate_plus, attempting to do a drush migrate-status (which reports available source counts, among other things) breaks due to the exception
SQLSTATE[HY000]: General error: 1366 Incorrect integer value: 'cache' for column 'expire' at row 1: INSERT INTO {cache_migrate} (cid, created, expire, tags, checksum, data, serialized) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6); Array
(
[:db_insert_placeholder_0] => 53bb9b0b38ac812554abeb88ea6d56760d07ec147a75e8761c4f57ecc298218f
[:db_insert_placeholder_1] => 1432158458.659
[:db_insert_placeholder_2] => cache
[:db_insert_placeholder_3] =>
[:db_insert_placeholder_4] => 0
[:db_insert_placeholder_5] => i:2430;
[:db_insert_placeholder_6] => 1
)
SourcePluginBase caches counts with
$this->getCache()->set($this->cacheKey, $count, 'cache');
But the signature on CacheBackendInterface::set() is
public function set($cid, $data, $expire = Cache::PERMANENT, array $tags = array());
I'm not sure why this is just becoming apparent now, the last change to that interface was January, but it's definitely broken.
Comment | File | Size | Author |
---|---|---|---|
#11 | interdiff.txt | 1.36 KB | mikeryan |
#11 | migration_count_caching-2492429-11.patch | 2.31 KB | mikeryan |
| |||
#11 | migration_count_caching-2492429-11-FAIL.patch | 1.23 KB | mikeryan |
Comments
Comment #1
mikeryanThis really needs a fail test (why is the existing cached source count test not failing?), but capturing the fix that I need right now...
Comment #2
mikeryanOK, so the problem here is SourcePluginBase passing a string where the cache set() interface expects an integer. The reason the tests didn't pick this up is because they're using NullBackend, which doesn't do anything with the bad argument. ApcuBackend looks like the simplest cache backend that will choke on the bad argument.
Comment #4
mikeryanResults as expected - ready for manual review.
Comment #5
phenaproximaLooks good to me, and it's green. I approve.
Comment #6
alexpottYep this is just broken. Committed c523c99 and pushed to 8.0.x. Thanks!
Comment #8
neclimdulThis forces a requirement on APCU for tests to pass. We need to do some sort of mock or @require.
Comment #10
alexpottI've revert this so we can do the necessary mocking to avoid #8.
Comment #11
mikeryanHere we go, mocking the cache interface so we can validate we're setting proper arguments.
Comment #13
mikeryanYep, the one that was supposed to fail did - ready for manual (re)review.
Comment #14
neclimdulinterdiff looks great, works without apcu, RTBC.
Comment #15
webchickCommitted and pushed to 8.0.x. Thanks!