Oh - I was expecting to see them at the top since that is where new changes are generally added.
Ralph On Jul 20, 2013, at 4:23 PM, Nick Williams wrote: > I did update changes.xml. Not sure why you don't see it, but it's there: > > http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/src/changes/changes.xml?r1=1505102&r2=1505209&diff_format=h > > Nick > > On Jul 20, 2013, at 6:21 PM, Ralph Goers wrote: > >> I didn't see an update to changes.xml. Since this is a breaking change I >> would definitely expect to see it in the list of changes. >> >> Ralph >> >> On Jul 20, 2013, at 2:58 PM, Nick Williams wrote: >> >>> 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 >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
