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 tables variable and cache* can't be imported from the backup SQL file.
CommentFileSizeAuthor
#8 demo_module.patch4.18 KBsmk-ka
dba.demo_.patch4.76 KBsun

Comments

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Reviewed & tested by the community » Needs review
jeremy’s picture

jeremy commented
Status: Needs review » Needs work

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?

jeremy’s picture

jeremy commented

The abstracted function you need to call is db_escape_string().

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

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.

mathieu’s picture

mathieu commented

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.

dww’s picture

dww
we/he/they
commented

blob escaping bug is now at http://drupal.org/node/154136

smk-ka’s picture

smk-ka commented
Assigned: sun » smk-ka
Category: task » feature
Status: Needs work » Needs review

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

smk-ka’s picture

smk-ka commented
StatusFileSize
new demo_module.patch4.18 KB

...and the patch.

jeremy’s picture

jeremy commented
Status: Needs review » Closed (won't fix)

The 5.x branch is no longer supported.