Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs review
FileSize
4.22 KB

Here is a initial version, it drops the current collection and create a new capped one

hurleyit’s picture

Component: Code » Cache
FileSize
4.22 KB

I tried out this patch. And I think there are a few issues with it but I ran into two when testing this patch. The term 'mongo' is misspelled mondo in a function name, mondodb_watchdog_settings, in mongodb_watchdog.admin.inc, and in a reference to the function in mongodb_watchdog_menu in mongodb_watchdog.module. I am attaching a revised patch that uses the correct spelling of mongo.

The other issue is that part of the patch to mongodb_watchdog.info failed.

patching file mongodb_watchdog.info
Hunk #1 FAILED at 1.
Hunk #2 FAILED at 7.
2 out of 2 hunks FAILED -- saving rejects to file mongodb_watchdog.info.rej
***************
*** 1,5 ****
  ;$Id: mongodb_watchdog.info,v 1.1 2009/11/29 22:42:13 dereine Exp $
- name = MongoDB watchdog
  description = 'A watchdog implementation using MongoDB'
  package = MongoDB
  version = VERSION
--- 1,5 ----
  ;$Id: mongodb_watchdog.info,v 1.1 2009/11/29 22:42:13 dereine Exp $
+ name = MongoDB Watchdog
  description = 'A watchdog implementation using MongoDB'
  package = MongoDB
  version = VERSION
*************** core = 7.x
*** 7,10 ****
  dependencies[] = mongodb
  files[] = mongodb_watchdog.module
  files[] = mongodb_watchdog.admin.inc

--- 7,11 ----
  dependencies[] = mongodb
  files[] = mongodb_watchdog.module
  files[] = mongodb_watchdog.admin.inc
+ files[] = mongodb_watchdog.test

Other potential issues:
1. I'm not sure it's wise to create another settings page for the MongoDB settings. These settings should probably appear with the other MongoDB Watchdog settings on the page admin/config/development/logging.
2. There is already a cap setting for the size of the Watchdog log, with the number of entries. Do we need to use a separate variable that is Mongo specific?
3. The patch assumes one is in the mongodb/mongodb_watchdog directory. My understanding, possibly flawed, is that it should assume placement in the mongodb directory instead.

hurleyit’s picture

I created a new patch that adds the form elements to the existing MongoDB Watchdog page instead of creating a new one. This way the form controls are all in one place instead of being spread out among different pages. It does not attempt to reconcile the issue of two controls for setting the number of lines to save.

hurleyit’s picture

There are some issues I'm still working out. Don't use my revised-2 patch for now.

hurleyit’s picture

I prepended the mongodb watchdog callback to the submit array instead of adding it to the end. This allows us to check if the user changed the capped settings during this submit. If there is a better way of doing this, I am more than willing to revise.

crea’s picture

Component: Cache » Watchdog

Doesn't apply to cache.

fgm’s picture

Status: Needs review » Needs work

No longer applies (CVS format patch ?)

fgm’s picture

Status: Needs work » Needs review

Note that a different version supporting capped collections was committed by chx on 2011-04-24 with no issue#, and message "Watchdog cleanup. Use $natural, implement clean log."

So maybe this issue is now fixed. Anyone willing to review ?

Status: Needs review » Needs work

The last submitted patch, mongodb_watchdog-capped-revised2a.patch, failed testing.

chx’s picture

Status: Needs work » Fixed

Yes this is fixed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.