File-Backup should not backup itself && watchdog-calls wrong in files_backup
| Project: | DB Maintenance |
| Version: | 6.x-2.0-beta1 |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
Jump to:
When the backup target-location is part of the Drupal-directory (e.g. drupal/sites/default/backup), the backup-script backups all files - including the backups. This leads to fast growing backupfiles (e.g. 1GB, 2GB, 4GB, 8GB, etc).
For several reasons (e.g. Access to backups through normal Drupal-modules like filebrowser) backuping to a drupal-path seems logical to me. Nevertheless "backuping backups" is strange. The given patch excludes all files matching $dbname_files.tar.gz and $dbname_db.sql.
A warning: Setting /tmp as the default is very, very unsecure. In most cases we want a backup after e.g. a server failure occured - and in most cases, /tmp will be deleted on a regular schedule and/or with every restart.
While patching the tar-command, I found two additional errors:
1) exec($command,$output) creates $output as an array. Thus implode('
',$output) should be called to get the "line-wise" output of $command
2) Some calls to watchdog(..) are wrong.
The applied patch fixes all the above. To avoid the "exclude backup-files"-patch, simply remove the patch to $command. It would be better to add an option that allows to exclude backups.
It would be nice to integrate db_maintenance with backup-migrate (though both modules exclude each others backupfiles).
| Attachment | Size |
|---|---|
| db_maintenance.patch | 2.79 KB |

#1
I apologize, priority should be critical as logging seems like an integral part of this module to me.
BTW: Writing $output=implode('
',$output); like I have done in the patch is somehow ugly and was only for testing purposes. Seems like I have diffed against the wrong file..
#2
This is still open - e.g. db_maintenance.module, line 475 - why is there a (wrong) watchdog-statement BEHIND a return-statement? The watchdog is never reached (so probably sensible to have) - either delete it or correct it and swap statements.
For the rest of the (unfixed and uncommented) problems - see above issue.
I am more then willing to provide a patch against whatever CVS-version needed - but I want to know whether it will be used beforehand.
I am also more then willing to simply patch CVS myself if you want me too.
#3
committed
#4
Automatically closed -- issue fixed for 2 weeks with no activity.