I just found an error in two of the migration functions.
In "sites/all/modules/contrib/backup_migrate/backup_migrate.module" we have the following two functions:
backup_migrate_perform_backup() and backup_migrate_perform_restore().
First of all they are coded inconsistent.
In the "backup_migrate_perform_backup()" there is the following code:
if (!ini_get('safe_mode') && strpos(ini_get('disable_functions'), 'set_time_limit') === FALSE && ini_get('max_execution_time') < 1200) {
The "backup_migrate_perform_restore()" it's this:
if (!ini_get('safe_mode') && strpos(ini_get('disable_functions'), 'set_time_limit') === FALSE && ini_get('max_execution_time') < variable_get('backup_migrate_backup_max_time', 1200)) {
The incosistent is clearly visible.
I really do not know where this value "variable_get('backup_migrate_backup_max_time')" should come from, but I can not find it anywhere.
Either way it should be included in both functions.
Also there is a problem with the php command line interface which is used by drush. The max_execution_time is always 0 so it is always lower than 1200. So it will be overwritten.
There should be a check for unlimited time in there.
Regards
func0der
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | backup-migrate-cli-max-execution-time-2103359-1.patch | 1.41 KB | johnnybgoode |
| #1 | backup_migrate_drush_timeout_fix.patch | 2.01 KB | func0der |
Comments
Comment #1
func0der commentedI corrected the version. Sorry, thought of it as the Drupal version.
Attached patch to speed this up. It is the same as I already mentioned above.
Comment #2
johnnybgoode commentedPatch to compare ini_get('max_execution_time') against the "backup_migrate_backup_max_time" variable instead of a hard-coded value in "backup_migrate_perform_backup()", and to check for cli invocation in both "backup_migrate_perform_backup()" and "backup_migrate_perform_restore()".
Comment #3
johnnybgoode commentedfunc0der - I didn't see that you had already submitted a patch. The only difference between yours and mine is that instead of checking if the value of 'max_execution_time' is 0, I am using the drupal_is_cli() function. This allows backup_migrate to still set a maximum time limit if someone attempts to perform a backup on a server with an unreasonable (or no) time limit set in php.ini. Of course the 'backup_migrate_backup_max_time' variable can be used to override the default time limit of 1200 seconds if someone still wants to run large backups from the web interface.
Comment #4
func0der commentedNo problem.
I did not know about the method you used. I am fine with that, of course. Yours is the better solution.
I did set the version wrong, you set it back, I am setting it back again to the newest one, because in there we have that bug. I confused the "version" field with the version of drupal at first.
Thanks for taking a look into this.
Regards
func0der
Comment #5
ronan commentedI've cleaned this up. Thanks!