Make auto_backup callable and dba less verbose
sun - March 31, 2007 - 00:35
| Project: | Database Administration |
| Version: | 5.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | smk-ka |
| Status: | needs review |
Description
I'm the maintainer of Demonstration Site (Sandbox) that uses several functions of dba to create a site snapshot and reset a site to that snapshot.
Based on the 5.x port I'm attaching a patch that I need to get in to make Demo module work properly.
In particular, this patch
- adds the ability to call
dba_auto_back()with custom function arguments. - makes
dba_get_fields()conditionally less verbose. - makes
dba_drop_table()conditionally less verbose. - fixes a bug in
dba_backup_table()that did not dump BLOBs and LONGBLOBs in proper binary format (I've copied the bin2hex() method that phpMyAdmin uses for such exports). Without this fix, some rows in the tablesvariableandcache*can't be imported from the backup SQL file.
| Attachment | Size |
|---|---|
| dba.demo_.patch | 4.76 KB |
#1
#2
I'd like to see the blob fix in a standalone issue. It's a bug that does need to be fixed. In that patch, however, I don't like the direct call to a mysql_() function. We should push to get that function included in Drupal's abstraction layer -- and until that happens, we should conditionally call the appropriate mysql or postgresql function.
(I'm well aware there's already mysql-specific logic in this module, but that's something I want to see fixed.)
As for the conditionals, are the absolutely necessary for what you're doing? They complicate the code, and my goal is to try and simplify things. Is there perhaps another way?
#3
The abstracted function you need to call is db_escape_string().
#4
The blob fix is now part of the 5.x port and already refactored there. After comparing our methods with the output of a phpMyAdmin database export, I've found out that we need to escape only certain characters. Otherwise the generated dumps can not be imported without SQL errors. It doesn't use the mysql-specific function anymore.
I need those conditional function arguments to call the dba functions from Demo.module. I really do not want to copy (fork) that functionality in Demo, because I believe that both modules will benefit from this reutilisation.
However, it seems that dba_get_fields() and dba_describe_tables() are somewhat similar now and could be merged into one function. But that's another issue, IMHO.
I'll post a new patch after the 5.x port has been committed to HEAD.
#5
Subscribing to issue. I did everything I could to get dba/demo working with Drupal 5.1 and this issue seems to be the only that's unresolved.
#6
blob escaping bug is now at http://drupal.org/node/154136
#7
I'd like to pick this one up, to have an out-of-the-box working Demo module some time in the near future.
This updated patch removes the separate functions arguments, and replaces them with a single, associative array. This array is then merged with the module's defaults. IMO this is a much cleaner, more drupalish way than in the original patch.
Additionally, this patch removes support for PHP versions lower than 4.2, because Drupal requires 4.3.3 according to the requirements page.
--
Stefan Kudwien
unleashed mind
#8
...and the patch.