You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Simon Kitching <si...@ecnetwork.co.nz> on 2003/10/27 23:17:16 UTC

[digester] plugins: patch for logging

Hi,

Robert: thanks for committing the plugins Exception patch.

Here is a patch to convert the Plugins module logging to always use the
Log object returned by digester.getLogger().

I still think that the way Digester centralizes logging is flawed.
However I have more interest in getting plugins finished than debating
logging, so this patch should resolve any arguments for the moment.

Maybe sometime later I will think about the logging issue and put
forward a more concrete proposal. At least now I think I understand the
requirements that led to the current implementation.

As the governor of California would say, "I'll be back".. :-)

Regards,

Simon

PS: I hope I've got the right licence header on new file LogUtils.java
this time!

Re: [digester] plugins: patch for logging

Posted by Jean-Francois Arcand <jf...@apache.org>.

Simon Kitching wrote:

>On Thu, 2003-10-30 at 02:43, Simon Kitching wrote:
>
>  
>
>>Any logging changes (if something can be agreed on) might go in the mix
>>too.
>>    
>>
>
>And Remy Maucherat's variable substitution stuff, if it's ready?
>
>And Jean-Francois Arcand's changes to schema validation?
>
J2EE 1.4 is mostly FCS so I will have time to work on my proposal 
probably by the end of the week. More to come....

-- Jeanfrancois


>
>I'm not aware of anything else that's been proposed but not committed...
>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
>  
>


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] plugins: patch for logging

Posted by "Craig R. McClanahan" <cr...@apache.org>.
Simon Kitching wrote:

>On Thu, 2003-10-30 at 02:43, Simon Kitching wrote:
>
>  
>
>>Any logging changes (if something can be agreed on) might go in the mix
>>too.
>>    
>>
>
>And Remy Maucherat's variable substitution stuff, if it's ready?
>
>And Jean-Francois Arcand's changes to schema validation?
>
>  
>
General +0 on a Digester release after these things are incorporated ... 
unfortunately I'm still a ways away from having time to contribute 
directly very much :-(.

>I'm not aware of anything else that's been proposed but not committed...
>
>  
>

Craig



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] plugins: patch for logging

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
i'd also like to use maven to generate documentation from simon's examples.
  i'd like to use this on the website and as part of the docs for the 
release.

- robert

On Wednesday, October 29, 2003, at 05:50 AM, Simon Kitching wrote:

> On Thu, 2003-10-30 at 02:43, Simon Kitching wrote:
>
>>
>> Any logging changes (if something can be agreed on) might go in the mix
>> too.
>
> And Remy Maucherat's variable substitution stuff, if it's ready?
>
> And Jean-Francois Arcand's changes to schema validation?
>
> I'm not aware of anything else that's been proposed but not committed...
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] plugins: patch for logging

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Wednesday, October 29, 2003, at 05:50 AM, Simon Kitching wrote:

<snip>

> I'm not aware of anything else that's been proposed but not committed...

there's the processing instruction stuff which i'd like to take a look at.
  but i don't seem to have much time at the moment and really want to fix 
up betwixt so that i can do a 0.5 release. so, i'd be happy to wait until 
after the release.

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] plugins: patch for logging

Posted by Simon Kitching <si...@ecnetwork.co.nz>.
On Thu, 2003-10-30 at 02:43, Simon Kitching wrote:

> 
> Any logging changes (if something can be agreed on) might go in the mix
> too.

And Remy Maucherat's variable substitution stuff, if it's ready?

And Jean-Francois Arcand's changes to schema validation?

I'm not aware of anything else that's been proposed but not committed...


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] plugins: patch for logging

Posted by Simon Kitching <si...@ecnetwork.co.nz>.
Hi,

Regarding logging in Digester; I may have found a solution that will
keep all parties happy. Let me know what you think....

Currently this class exists in the plugins module to handle logging:

public class LogUtils {
  public static Log getLogger(Digester digester) {
    if (digester == null) {
        return new org.apache.commons.logging.impl.NoOpLog();
    }
    
    return digester.getLogger();
  }
}

If it was changed to the code below, promoted to the main digester
package, and used by all code, then:
(a) 
For applications which never call Digester.setLogger(...), logging would
be done via different categories, which would make me happy, and
(b)
For applications which call Digester.setLogger(log) things would work as
currently; all logging for all related objects gets redirected to a
single Log object (ie a single category). I think this sucks, but as my
applications don't redirect logging, I don't care.
(c)
It would be 100% backwards-compatible as far as I can see. No API change
occurs on the Digester (users can still call Digester.setLogger as
before). Code which directly accesses


public class LogUtils {
  /** default log category is specified string */
  public static Log getLogger(Digester digester, String dfltCategory) {
    if (digester == null) {
        return new org.apache.commons.logging.impl.NoOpLog();
    }
    else {
      Log log = digester.getLogger();
      if (log == null) {
        log = LogFactory.getLog(dfltCategory)
      }
    return log;
  }

  /** default log category is class name */
  public static Log getLogger(Digester digester, Class klass) {
    ...
  }
}


As icing on the cake, I suggest the RulesBase class be modified to
manage the log object.
The code below sets the log up as a NoOpLog initially, then when the
digester object is set, it changes the log object to either the
user-specified log object or a log category for the class.

class RulesBase .. {
 protected Log log;

 public RulesBase() {
   log = LogUtils.getLogger(null, this.getClass());
 }

 public void setDigester(Digester d) {
   log = LogUtils.getLogger(d, this.getClass())
 }
}

This has a *slightly* different effect from my usual convention of
having a static private Log object on each class; with the code above,
the log category used for logging from inherited methods comes out with
the category of the derived class, not the category of the class on
which the method was actually declared. However that's close enough for
me...

In addition, making the RulesBase object keep track of the log object
means that derived classes are insulated from the actual details of how
the log object is derived. And the implementation is fairly efficient; a
lookup of log object is only done once at construction, and once in
setDigester.

There is a constraint on the above: it is that Digester.setLog must be
called before any objects (such as Rule objects) are added to the
digester. Changing the Log object later may not have the desired effect.
This seems reasonable to me, and may have been the original intention:
it's not explicitly specified, though.

Regards,

Simon



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] plugins: patch for logging

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Wednesday, October 29, 2003, at 12:43 AM, Simon Kitching wrote:

<snip>

> Once the new release is out, I would like to start looking at some of
> the other apache projects to determine which of them don't use Digester,
> and why. Maybe we can then roll extra functionality they need into the
> base Digester. In particular, Ceki Gulcu (log4j) has a sandbox project
> Joran, which sounds very digester-like. I think it would be great if we
> could merge the projects. Any comments?

there are good reasons (to do with commons-logging) why ceki can't re-use 
the digester code base for log4j so i heard that he was creating a minimal,
  stripping down engine inspired by digester design ideas. (you'd need to 
ask him whether this became joran.)

> BTW, I'm currently learning Ruby, and decided to practice by porting
> Digester to Ruby. It's coming along very well (and turned out to be an
> excellent learning experience). Look for "xmldigester" coming to
> RubyForge soon! Any Rubyish types watching this email list are welcome
> to join in. I'll post an email here when the code is up, or you can
> contact me directly.

cool. i've always wanted to learn python and create a digester port but i'
ve never found the time. it'd be good if you could keep xmldigester under 
an ASF compatible license just in case ;)

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] plugins: patch for logging

Posted by Simon Kitching <si...@ecnetwork.co.nz>.
On Wed, 2003-10-29 at 12:34, robert burrell donkin wrote:
> On Monday, October 27, 2003, at 10:17 PM, Simon Kitching wrote:
> 
> > Hi,
> 
> hi simon
> 
> i'm not totally convinced that your patch is the best possible approach 
> but i think that it's an improvement so i've committed it. it's pretty 
> much the baseline required for compatibility with the rest of digester. we 
> need another digester release sometime soonish so maybe this needs 
> thinking about sooner rather than later.

Aha! Good to see that you're thinking about logging too :-)

By the "not convinced" comment, do you mean that I am not complying with
the current Digester conventions correctly, or do you mean that the
current Digester conventions aren't right?

Regarding a new release, I think it would be nice to get something out
before Christmas. I've seen lots of small fixes go in over the last few
months.

Is plugins sufficiently accepted to become part of this new release?
I haven't heard anyone raise any objections to it, or suggest it should
be removed so I hope so.

What I would suggest is:

(a) 
I need to submit patches for:
 (1) Assertion --> RuntimeException (coming today)
 (2) setting attribute which specifies plugin, eg
       <foo classe-de-plugin="..."/>
 (3) A change to the way "autoSetProperties" works.
     I want to have a pluggable "strategy" parameter
     instead of a boolean, for more flexibility.

(b) 
If no-one has any more issues with plugins, I will port my (ECN's)
commercial app over to the new library and run tests.
This will be a good test of both the API and the implementation.
This should take only a day of actual work, but with my current
workload, it will probably take about 1 week elapsed time.

Of course I only want to do this if there is a pretty good chance that
plugins will stay in Digester....

(c) 
Do a release, or release candidate.

Any logging changes (if something can be agreed on) might go in the mix
too.

On a related note, I went through the outstanding digester
bugs/enhancements in buzgilla a while ago and added some comments.
Unfortunately nothing happened. I think that almost all the outstanding
entries can be closed, which would be cool to do before the next
release.


> nothings every finished :)

Good point!

> i've been thinking about this for a little while and i think that i now 
> have some ideas about the way i'd prefer logging to work which should 
> preserve backwards compatibility. (i'd like to put setters and getters for 
> the logs on each class and then set then from parent to child as they are 
> created. )

I'm not sure this will work. In order to have a proper hierarchy of
categories, the same Log object can't just be passed to the child.

I've been thinking about enhancing commons-logging so that a LogFactory
object can be placed in thread-local storage, and the static LogFactory
method will look there first, and delegate if there is one.

For containers which dedicate a thread to a "subsystem", this would
work. For containers which use thread pools, the container would need to
install the correct LogFactory object before using the thread to do work
in a specific contained app. Yes, that's logging-specific work. But so
is calling the setLog method on Digester or similar.

Well, those are just initial thoughts. I was intending to think it
through a bit more before suggesting any ideas, but as you are looking
at logging, I had better put it forward now..

> 
> > As the governor of California would say, "I'll be back".. :-)
> 
> i'll look forward to that. i'd certainly like to encourage you to stay 
> involved with digester.

I'd be happy to. 

Once the new release is out, I would like to start looking at some of
the other apache projects to determine which of them don't use Digester,
and why. Maybe we can then roll extra functionality they need into the
base Digester. In particular, Ceki Gulcu (log4j) has a sandbox project
Joran, which sounds very digester-like. I think it would be great if we
could merge the projects. Any comments?

BTW, I'm currently learning Ruby, and decided to practice by porting
Digester to Ruby. It's coming along very well (and turned out to be an
excellent learning experience). Look for "xmldigester" coming to
RubyForge soon! Any Rubyish types watching this email list are welcome
to join in. I'll post an email here when the code is up, or you can
contact me directly.

Regards,

Simon


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Re: [digester] plugins: patch for logging

Posted by robert burrell donkin <ro...@blueyonder.co.uk>.
On Monday, October 27, 2003, at 10:17 PM, Simon Kitching wrote:

> Hi,

hi simon

i'm not totally convinced that your patch is the best possible approach 
but i think that it's an improvement so i've committed it. it's pretty 
much the baseline required for compatibility with the rest of digester. we 
need another digester release sometime soonish so maybe this needs 
thinking about sooner rather than later.

> Robert: thanks for committing the plugins Exception patch.
>
> Here is a patch to convert the Plugins module logging to always use the
> Log object returned by digester.getLogger().
>
> I still think that the way Digester centralizes logging is flawed.
> However I have more interest in getting plugins finished than debating
> logging, so this patch should resolve any arguments for the moment.

nothings every finished :)

at leastways, no open source code ever is. the only problem is energy (but 
i guess that's down to the second law of thermodynamics).

> Maybe sometime later I will think about the logging issue and put
> forward a more concrete proposal. At least now I think I understand the
> requirements that led to the current implementation.

i've been thinking about this for a little while and i think that i now 
have some ideas about the way i'd prefer logging to work which should 
preserve backwards compatibility. (i'd like to put setters and getters for 
the logs on each class and then set then from parent to child as they are 
created. )

> As the governor of California would say, "I'll be back".. :-)

i'll look forward to that. i'd certainly like to encourage you to stay 
involved with digester.

> Regards,
>
> Simon
>
> PS: I hope I've got the right licence header on new file LogUtils.java
> this time!

did i miss one the other time?

(i'm not convinced that there aren't a few more typos in the licenses but 
these are common to all of digester so i'll probably end up running some 
more of my scripts against them.)

- robert


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org