The conversion from
"handleExceptions"/"suppressExceptions"/"isExceptionSuppressed" to
"ignoreExceptions" has been completed.
I still have some work to do to make sure these are being used/abided by
consistently and to make sure that all appenders and managers really do let
exceptions propagate.
Nick
On Jul 20, 2013, at 3:57 PM, Ralph Goers wrote:
> Yeah - that seems to work.
>
> Ralph
>
> On Jul 20, 2013, at 1:35 PM, Nick Williams wrote:
>
>> You left out the exclamation point.
>>
>> If value of "junk" is specified and the defaultValue is true then
>> (!"false".equalsIgnoreCase("junk") && true) returns true.
>>
>> However, it should actually be (defaultValue &&
>> !"false".equalsIgnoreCase(s)) so that it short-circuits it defaultValue is
>> false.
>>
>> Nick
>>
>> On Jul 20, 2013, at 3:30 PM, Ralph Goers wrote:
>>
>>> Why does this not look right to me? If a value of "junk" is specified and
>>> the defaultValue is true then ("false".equalsIgnoreCase("junk") && true)
>>> return false, which is incorrect. It should just return the default value.
>>>
>>> Ralph
>>>
>>> On Jul 20, 2013, at 1:18 PM, Nick Williams wrote:
>>>
>>>> Correction below:
>>>>
>>>> On Jul 20, 2013, at 2:38 PM, Nick Williams wrote:
>>>>
>>>>> Finally got back to working on this. Noticed two things:
>>>>>
>>>>> 1) On some appenders, ignoreExceptions/suppressExceptions defaults to
>>>>> true. On other ones it defaults to false. We should be consistent in
>>>>> this, and IMO it should default to true. Does anyone have any objection
>>>>> to that?
>>>>>
>>>>> 2) o.a.l.l.core.helpers.Booleans.parseBoolean may or may not be working
>>>>> as expected, so I wanted to check with y'all. Here's the code:
>>>>>
>>>>> return Strings.isEmpty(s) ? defaultValue : Boolean.parseBoolean(s);
>>>>>
>>>>> Boolean.parseBoolean() doesn't have a way to specify a default value. It
>>>>> automatically defaults to false if the value passed in is anything other
>>>>> than "true." So, if someone passes in a non-empty value other than "true"
>>>>> or "false", it will always return false even if defaultValue is true. I
>>>>> propose changing it to this:
>>>>>
>>>>> return "true".equalsIgnoreCase(s) || defaultValue;
>>>>
>>>> return "true".equalsIgnoreCase(s) || (!"false".equalsIgnoreCase(s) &&
>>>> defaultValue);
>>>>
>>>>>
>>>>> Does anyone have any problem with that?
>>>>>
>>>>> Nick
>>>>>
>>>>> On Jul 18, 2013, at 8:12 PM, Gary Gregory wrote:
>>>>>
>>>>>> On Jul 18, 2013, at 20:28, Paul Benedict <[email protected]> wrote:
>>>>>>
>>>>>>> "Proper documentation" is the key phrase. :-)
>>>>>>
>>>>>> Yes! :)
>>>>>>
>>>>>> Gary
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Jul 18, 2013 at 7:25 PM, Nick Williams
>>>>>>> <[email protected]> wrote:
>>>>>>> Okay. Hold everything! Lol...
>>>>>>>
>>>>>>> I started working on this change and then realized something.
>>>>>>> "Suppress" means "to forcibly put an end to," "restrain," or "prevent
>>>>>>> the development, action, or expression of." However, "ignore" means
>>>>>>> "refuse to take notice of or acknowledge; disregard intentionally" or
>>>>>>> "fail to consider (something significant)."
>>>>>>>
>>>>>>> Though not guaranteed, I can see how a user would mistake the word
>>>>>>> "ignore" to mean that exceptions aren't even logged, while "suppress"
>>>>>>> would be more obvious to mean they aren't thrown (but also confusing
>>>>>>> with Java suppressed exceptions as others pointed out). I don't think
>>>>>>> it's a big deal because no matter what we do we're going to have to
>>>>>>> properly document this field. But I just wanted to make sure everyone
>>>>>>> had thought about this before we proceeded.
>>>>>>>
>>>>>>> Nick
>>>>>>>
>>>>>>> On Jul 18, 2013, at 6:58 PM, Nick Williams wrote:
>>>>>>>
>>>>>>> > Okay. I'll proceed with the change.
>>>>>>> >
>>>>>>> > Nick
>>>>>>> >
>>>>>>> > On Jul 18, 2013, at 2:08 PM, Ralph Goers wrote:
>>>>>>> >
>>>>>>> >> First, I appreciate having the discussion before a code change like
>>>>>>> >> this is made. If that had been done I probably would have vetoed it.
>>>>>>> >> But this is the ASF where everybody gets a single voice and a
>>>>>>> >> single vote. Although I dislike this change and the effect it will
>>>>>>> >> have on my users I won't veto it if that is what the majority wants.
>>>>>>> >>
>>>>>>> >> Ralph
>>>>>>> >>
>>>>>>> >> On Jul 18, 2013, at 11:06 AM, Nick Williams wrote:
>>>>>>> >>
>>>>>>> >>> Hmmm. So it sounds like we're at an impasse. Everyone except Ralph
>>>>>>> >>> seems to agree to renaming it ignoreExceptions, bug Ralph said the
>>>>>>> >>> below in opposition. Ralph, can you clarify a little? Are you
>>>>>>> >>> objecting to just renaming the XML/JSON attribute (which, really,
>>>>>>> >>> is the only thing that would break your teams' configurations)? Or
>>>>>>> >>> are you objecting to ignoreExceptions completely? I can't tell
>>>>>>> >>> whether you're okay with the isExceptionSuppressed method being
>>>>>>> >>> renamed.
>>>>>>> >>>
>>>>>>> >>> I would just throw it out there that, while I understand that it
>>>>>>> >>> would be inconvenient to have to modify all your configurations,
>>>>>>> >>> this IS beta software. It is not GA yet (hopefully next month or
>>>>>>> >>> so). Anyone who uses beta software does so with the understanding
>>>>>>> >>> that there could be breaking changes before GA. If you didn't want
>>>>>>> >>> to take that risk, I respectively submit that you shouldn't have
>>>>>>> >>> used beta software in production. I'm not saying, "You're wrong."
>>>>>>> >>> I'm just questioning the motives behind your opposition. In short:
>>>>>>> >>>
>>>>>>> >>> If we all agree that ignoreExceptions is a better name that
>>>>>>> >>> suppressExceptions (inconvenience aside), now is the time to change
>>>>>>> >>> it. We can't after GA.
>>>>>>> >>>
>>>>>>> >>> Now I'm not sure how to proceed. :-P
>>>>>>> >>>
>>>>>>> >>> Nick
>>>>>>> >>>
>>>>>>> >>> On Jul 18, 2013, at 1:02 AM, Ralph Goers wrote:
>>>>>>> >>>
>>>>>>> >>>> I do not have a problem with renaming handleExceptions to
>>>>>>> >>>> exceptionSuppressed. I do have a problem with renaming
>>>>>>> >>>> supressExceptions to ignoreExceptions, primarily because I have a
>>>>>>> >>>> bunch of teams using Log4j 2 in production and they would have to
>>>>>>> >>>> modify their configurations when they upgrade. Furthermore, I
>>>>>>> >>>> can't in my wildest dreams imagine anyone getting confused over
>>>>>>> >>>> this parameter and suppressed Throwables.
>>>>>>> >>>>
>>>>>>> >>>> FWIW - handleExceptions means that the Appender "handles" the
>>>>>>> >>>> exception (i.e. it is suppressed). I don't recall why the
>>>>>>> >>>> variables don't match - I think I might have originally exposed
>>>>>>> >>>> "handleExceptions" and found that to be ambiguous and renamed the
>>>>>>> >>>> config param but not the internal variable.
>>>>>>> >>>>
>>>>>>> >>>> Ralph
>>>>>>> >>>>
>>>>>>> >>>> On Jul 17, 2013, at 2:42 PM, Nick Williams wrote:
>>>>>>> >>>>
>>>>>>> >>>>> Appender specifies a method, isExceptionSuppressed(), which
>>>>>>> >>>>> indicates whether exceptions thrown while appending events should
>>>>>>> >>>>> be suppressed (logged instead of re-thrown).
>>>>>>> >>>>>
>>>>>>> >>>>> AbstractAppender implements this method with a private
>>>>>>> >>>>> handleExceptions field and a handleExceptions constructor
>>>>>>> >>>>> argument. isExceptionSuppressed() returns handleExceptions (so,
>>>>>>> >>>>> supposedly, "handle exceptions" means "take care of exceptions
>>>>>>> >>>>> instead of the user having to take care of exceptions").
>>>>>>> >>>>>
>>>>>>> >>>>> Everybody that extends AbstractAppender uses the same
>>>>>>> >>>>> handleExceptions constructor argument. They all define a
>>>>>>> >>>>> suppressExceptions XML attribute that is assigned to the
>>>>>>> >>>>> handleExceptions constructor argument in the static plugin
>>>>>>> >>>>> factory method.
>>>>>>> >>>>>
>>>>>>> >>>>> This is all very confusing to me. I just realize that I have
>>>>>>> >>>>> misunderstood "handleExceptions" this whole time in the database
>>>>>>> >>>>> appenders and have assumed it was the opposite of
>>>>>>> >>>>> isExceptionSuppressed() / suppressExceptions (and, thus, have
>>>>>>> >>>>> written incorrect code).
>>>>>>> >>>>>
>>>>>>> >>>>> Does anyone have a problem with me renaming handleExceptions to
>>>>>>> >>>>> exceptionSuppressed (to match the JavaBean isExceptionSuppressed
>>>>>>> >>>>> method) to make this less confusing?
>>>>>>> >>>>>
>>>>>>> >>>>> Nick
>>>>>>> >>>>> ---------------------------------------------------------------------
>>>>>>> >>>>> To unsubscribe, e-mail: [email protected]
>>>>>>> >>>>> For additional commands, e-mail: [email protected]
>>>>>>> >>>>>
>>>>>>> >>>>
>>>>>>> >>>>
>>>>>>> >>>> ---------------------------------------------------------------------
>>>>>>> >>>> To unsubscribe, e-mail: [email protected]
>>>>>>> >>>> For additional commands, e-mail: [email protected]
>>>>>>> >>>>
>>>>>>> >>>
>>>>>>> >>>
>>>>>>> >>> ---------------------------------------------------------------------
>>>>>>> >>> To unsubscribe, e-mail: [email protected]
>>>>>>> >>> For additional commands, e-mail: [email protected]
>>>>>>> >>>
>>>>>>> >>
>>>>>>> >>
>>>>>>> >> ---------------------------------------------------------------------
>>>>>>> >> To unsubscribe, e-mail: [email protected]
>>>>>>> >> For additional commands, e-mail: [email protected]
>>>>>>> >>
>>>>>>> >
>>>>>>>
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail: [email protected]
>>>>>>> For additional commands, e-mail: [email protected]
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Cheers,
>>>>>>> Paul
>>>>>
>>>>
>>>
>>
>
