Matt,
The only objection I have to builders is that there should only be one way to
configure plugins and I have neither the time or energy to convert all plugins
from factories to builders. With 130+ open issues I think our time is better
focused there instead of fixing something that already works.
And, FWIW, the only place you will see createAppender coded like that is in
unit tests and there are a bunch of ways to make that clearer.
Ralph
On Jun 15, 2014, at 7:19 PM, Matt Sicker <boa...@gmail.com> wrote:
> I'm really against using factory methods due to language limitations in Java.
> You can't specify default values, for one. Two, the more parameters a factory
> takes, the crazier the method is. Seriously, tell me what this method is
> specifying:
>
> FileAppender.createAppender("true", "true", "true", "true", "true", "true",
> "true", "4096", null, null, "false", null, null);
>
>
> On 15 June 2014 21:05, Remko Popma <remko.po...@gmail.com> wrote:
> I'm fine with just the factory methods too.
>
> Sent from my iPhone
>
> On 2014/06/16, at 9:44, Scott Deboy <scott.de...@gmail.com> wrote:
>
>> +1
>>
>> On Jun 15, 2014 4:05 PM, "Ralph Goers" <ralph.go...@dslextreme.com> wrote:
>> Do we need the builders? As I said, I prefer only one way for creating
>> plugins.
>>
>> Ralph
>>
>> On Jun 15, 2014, at 2:49 PM, Remko Popma <remko.po...@gmail.com> wrote:
>>
>>> I see. I agree that the original format is much nicer.
>>>
>>> Matt, do you think you can achieve this with the builders?
>>>
>>> Sent from my iPhone
>>>
>>> On 2014/06/16, at 1:29, Ralph Goers <ralph.go...@dslextreme.com> wrote:
>>>
>>>> While you improved some of the existing messages, you really didm’t
>>>> address what I wanted fixed. The previous debug logs would have had
>>>> messages similar to:
>>>>
>>>> Calling createLayout on class
>>>> org.apache.logging.log4j.core.layout.PatternLayout for element
>>>> PatternLayout with params(pattern="%d{HH:mm:ss.SSS} [%t] %-5level
>>>> %logger{36} - %msg%n",
>>>> Configuration(D:\rista\eclipsekws\.metadata\.plugins\org.eclipse.wst.server.core\tmp1\wtpwebapps\log4j2.0-test\WEB-INF\classes\test-log4j.xml),
>>>> null, charset="null", alwaysWriteExceptions="null")
>>>>
>>>> Calling createAppender on class
>>>> org.apache.logging.log4j.core.appender.ConsoleAppender for element Console
>>>> with params(PatternLayout(%d{HH:mm:ss.SSS} [%t] %-5level %logger{36} -
>>>> %msg%n), null, target="SYSTEM_OUT", name="console", follow="null",
>>>> ignoreExceptions="null")
>>>>
>>>> Calling createAppenderRef on class
>>>> org.apache.logging.log4j.core.config.AppenderRef for element appender-ref
>>>> with params(ref="console", level="null", null)
>>>> 2013-09-20 15:06:55,749 DEBUG Calling createLogger on class
>>>> org.apache.logging.log4j.core.config.LoggerConfig$RootLogger for element
>>>> root with params(additivity="null", level="error", includeLocation="null",
>>>> AppenderRef={console}, Properties={},
>>>> Configuration(D:\rista\eclipsekws\.metadata\.plugins\org.eclipse.wst.server.core\tmp1\wtpwebapps\log4j2.0-test\WEB-INF\classes\test-log4j.xml),
>>>> null)
>>>>
>>>> The current log emits stuff like:
>>>>
>>>> 2014-06-15 09:07:19,432 DEBUG Building Plugin[name=layout,
>>>> class=org.apache.logging.log4j.core.layout.XmlLayout]. Searching for
>>>> builder factory method...
>>>> 2014-06-15 09:07:19,435 DEBUG No builder factory method found in class
>>>> org.apache.logging.log4j.core.layout.XmlLayout.
>>>> 2014-06-15 09:07:19,435 DEBUG Still building Plugin[name=layout,
>>>> class=org.apache.logging.log4j.core.layout.XmlLayout]. Searching for
>>>> factory method...
>>>> 2014-06-15 09:07:19,436 DEBUG Found factory method [createLayout]: public
>>>> static org.apache.logging.log4j.core.layout.XmlLayout
>>>> org.apache.logging.log4j.core.layout.XmlLayout.createLayout(boolean,boolean,boolean,boolean,java.nio.charset.Charset).
>>>> 2014-06-15 09:07:19,436 DEBUG Generating parameters for factory method
>>>> [createLayout]...
>>>> 2014-06-15 09:07:19,456 DEBUG Attribute(locationInfo="false") - no value
>>>> specified, using default.
>>>> 2014-06-15 09:07:19,456 DEBUG Attribute(properties="false") - no value
>>>> specified, using default.
>>>> 2014-06-15 09:07:19,457 DEBUG Attribute(complete="true") - no value
>>>> specified, using default.
>>>> 2014-06-15 09:07:19,457 DEBUG Attribute(compact="false") - no value
>>>> specified, using default.
>>>> 2014-06-15 09:07:19,457 DEBUG Attribute(charset="UTF-8") - no value
>>>> specified, using default.
>>>> 2014-06-15 09:07:19,587 DEBUG Built Plugin[name=layout] OK from factory
>>>> method.
>>>> 2014-06-15 09:07:19,588 DEBUG Building Plugin[name=appender,
>>>> class=org.apache.logging.log4j.core.appender.FileAppender]. Searching for
>>>> builder factory method...
>>>> 2014-06-15 09:07:19,588 DEBUG No builder factory method found in class
>>>> org.apache.logging.log4j.core.appender.FileAppender.
>>>> 2014-06-15 09:07:19,589 DEBUG Still building Plugin[name=appender,
>>>> class=org.apache.logging.log4j.core.appender.FileAppender]. Searching for
>>>> factory method...
>>>> 2014-06-15 09:07:19,589 DEBUG Found factory method [createAppender]:
>>>> public static org.apache.logging.log4j.core.appender.FileAppender
>>>> org.apache.logging.log4j.core.appender.FileAppender.createAppender(java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String,org.apache.logging.log4j.core.Layout,org.apache.logging.log4j.core.Filter,java.lang.String,java.lang.String,org.apache.logging.log4j.core.config.Configuration).
>>>> 2014-06-15 09:07:19,589 DEBUG Generating parameters for factory method
>>>> [createAppender]...
>>>> 2014-06-15 09:07:19,595 DEBUG
>>>> Attribute(fileName="target/XmlCompleteFileAppenderTest.log")
>>>> 2014-06-15 09:07:19,596 DEBUG Attribute(append="false")
>>>> 2014-06-15 09:07:19,596 DEBUG Attribute(locking="null")
>>>> 2014-06-15 09:07:19,596 DEBUG Attribute(name="XmlFile")
>>>> 2014-06-15 09:07:19,597 DEBUG Attribute(immediateFlush="false")
>>>> 2014-06-15 09:07:19,597 DEBUG Attribute(ignoreExceptions="null")
>>>> 2014-06-15 09:07:19,597 DEBUG Attribute(bufferedIo="null")
>>>> 2014-06-15 09:07:19,598 DEBUG Attribute(bufferSize="null")
>>>> 2014-06-15 09:07:19,598 DEBUG
>>>> XMLLayout(org.apache.logging.log4j.core.layout.XmlLayout@5eef9f84)
>>>> 2014-06-15 09:07:19,598 DEBUG Attribute(advertise="null")
>>>> 2014-06-15 09:07:19,599 DEBUG Attribute(advertiseUri="null")
>>>> 2014-06-15 09:07:19,599 DEBUG
>>>> Configuration(/Users/rgoers/projects/apache/logging/log4j/log4j2/trunk/log4j-core/target/test-classes/XmlCompleteFileAppenderTest.xml)
>>>> 2014-06-15 09:07:19,601 DEBUG Starting FileManager
>>>> target/XmlCompleteFileAppenderTest.log
>>>>
>>>> The previous format was a lot more compact as it essentially showed you
>>>> the parameters being passed to the factory method in one line while
>>>> identifying the class it came from and the configuration element that
>>>> triggered it. The new log emits that info as individual lines with a few
>>>> messages that are just noise.
>>>>
>>>> Ralph
>>>>
>>>>
>>>>
>>>>
>>>> On Jun 14, 2014, at 10:11 PM, Remko Popma <remko.po...@gmail.com> wrote:
>>>>
>>>>> I've done some work on this, there may be more places to improve, I
>>>>> mainly focused on PluginBuilder and PluginAttributeVisitor.
>>>>>
>>>>>
>>>>>
>>>>> On Thu, Jun 12, 2014 at 2:09 AM, Matt Sicker <boa...@gmail.com> wrote:
>>>>> Yeah, I liked the prettier logging format. I was planning to add it back
>>>>> in, but it appears as though I completely forgot about it! The "new"
>>>>> format was a quick placeholder. I'll try and work on that this week.
>>>>>
>>>>>
>>>>> On 10 June 2014 19:47, Gary Gregory <garydgreg...@gmail.com> wrote:
>>>>> Maybe Matt can shed a light on this?
>>>>>
>>>>> Gary
>>>>>
>>>>>
>>>>> On Tue, Jun 10, 2014 at 8:43 PM, Ralph Goers <rgo...@apache.org> wrote:
>>>>> I don't know exactly what I would be vetoing. I have no problem with
>>>>> some of the refactoring. The commit(s) that changed the logging probably
>>>>> happened weeks ago and I am just noticing now.
>>>>>
>>>>> But yes, I want the logging aspect of the changes reverted back to what
>>>>> was previously being done.
>>>>>
>>>>> Sent from my iPad
>>>>>
>>>>> On Jun 10, 2014, at 5:34 PM, Gary Gregory <garydgreg...@gmail.com> wrote:
>>>>>
>>>>>> Well, for Log4j Plugins, one way to configure should be plenty. I am OK
>>>>>> with the factory method pattern, while it makes for some long
>>>>>> signatures, I like that it is all in one place.
>>>>>>
>>>>>> May I suggest a simple "-1" reply on the ML on the changes to logging?
>>>>>> Do you feel a VETO is inappropriate here?
>>>>>>
>>>>>> Gary
>>>>>>
>>>>>>
>>>>>> On Tue, Jun 10, 2014 at 7:57 PM, Ralph Goers <rgo...@apache.org> wrote:
>>>>>> I think the discussion was not on its own thread. I thought there was
>>>>>> agreement that there should be only one way to configure plugins. I
>>>>>> prefer the factory method simply because it would be a whole lot of
>>>>>> effort to convert everything to a builder and I just don't think the
>>>>>> benefit is worth the effort.
>>>>>>
>>>>>> I spent a lot of time making the debug output "nice" and easily
>>>>>> understandable so I am a bit upset that it was tossed and replaced with
>>>>>> what you see below.
>>>>>>
>>>>>> Ralph
>>>>>>
>>>>>> Sent from my iPad
>>>>>>
>>>>>> On Jun 10, 2014, at 4:31 PM, Gary Gregory <garydgreg...@gmail.com> wrote:
>>>>>>
>>>>>>> On Tue, Jun 10, 2014 at 7:11 PM, Ralph Goers
>>>>>>> <ralph.go...@dslextreme.com> wrote:
>>>>>>> I am working on a new Appender and am noticing that the debug output is
>>>>>>> now far less useful than it used to be. I used to see the factory
>>>>>>> method being invoked with all of its parameters very nicely formatted.
>>>>>>> Now I see
>>>>>>>
>>>>>>> 2014-06-10 16:02:37,858 DEBUG No compatible method annotated with
>>>>>>> interface
>>>>>>> org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory found
>>>>>>> in class class org.apache.logging.log4j.web.appender.ServletAppender.
>>>>>>> 2014-06-10 16:02:37,858 DEBUG Found factory method class
>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender.public static
>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender
>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender.createAppender(org.apache.logging.log4j.core.Layout,org.apache.logging.log4j.core.Filter,java.lang.String,java.lang.String).
>>>>>>> 2014-06-10 16:02:37,864 DEBUG Constructing plugin of type class
>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender
>>>>>>> 2014-06-10 16:02:37,864 DEBUG PatternLayout(%m%n)
>>>>>>> 2014-06-10 16:02:37,864 DEBUG Constructing plugin of type class
>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender
>>>>>>> 2014-06-10 16:02:37,865 DEBUG Constructing plugin of type class
>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender
>>>>>>> 2014-06-10 16:02:37,865 DEBUG Attribute(name="Servlet")
>>>>>>> 2014-06-10 16:02:37,865 DEBUG Constructing plugin of type class
>>>>>>> org.apache.logging.log4j.web.appender.ServletAppender
>>>>>>> 2014-06-10 16:02:37,865 DEBUG Null string given to convert. Using
>>>>>>> default [null].
>>>>>>> 2014-06-10 16:02:37,866 DEBUG Attribute(ignoreExceptions="null")
>>>>>>>
>>>>>>> This is far more verbose, repetitive, and is nowhere near as clear as
>>>>>>> it used to be.
>>>>>>>
>>>>>>> Can you please get the logging output back to the old format?
>>>>>>>
>>>>>>> Also, can we change PatternLayout back to a factory and remove the
>>>>>>> message about no builder factory being present?
>>>>>>>
>>>>>>> I think we need to decide how many ways there are to configure a
>>>>>>> plugin: factory, builder, and whatever else we've been discussing. This
>>>>>>> is getting quite confusing!
>>>>>>>
>>>>>>> I thought we had a thread going on the topic already...
>>>>>>>
>>>>>>> Gary
>>>>>>>
>>>>>>>
>>>>>>> Ralph
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
>>>>>>> Java Persistence with Hibernate, Second Edition
>>>>>>> JUnit in Action, Second Edition
>>>>>>> Spring Batch in Action
>>>>>>> Blog: http://garygregory.wordpress.com
>>>>>>> Home: http://garygregory.com/
>>>>>>> Tweet! http://twitter.com/GaryGregory
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
>>>>>> Java Persistence with Hibernate, Second Edition
>>>>>> JUnit in Action, Second Edition
>>>>>> Spring Batch in Action
>>>>>> Blog: http://garygregory.wordpress.com
>>>>>> Home: http://garygregory.com/
>>>>>> Tweet! http://twitter.com/GaryGregory
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
>>>>> Java Persistence with Hibernate, Second Edition
>>>>> JUnit in Action, Second Edition
>>>>> Spring Batch in Action
>>>>> Blog: http://garygregory.wordpress.com
>>>>> Home: http://garygregory.com/
>>>>> Tweet! http://twitter.com/GaryGregory
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Matt Sicker <boa...@gmail.com>
>>>>>
>>>>
>>
>
>
>
> --
> Matt Sicker <boa...@gmail.com>
