You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4j-dev@logging.apache.org by Nick Williams <ni...@nicholaswilliams.net> on 2013/04/27 13:46:07 UTC
Appender close() vs stop()
While working on database appenders, I figured that the best place to "connect" is in Appender#start() and, likewise, the best place to "disconnect" is in Appender#stop(). However, then I noticed that AbstractAppender defines a close() method with the JavaDoc comment "Close the stream associated with the Appender." This could be read "Close the database connection associated with the Appender."
So, with that said, where should I close the connection? In stop() or in close()? The AbstractOutputStreamAppender closes the stream in stop() (which is what I originally expected) and never overrides close(). Since close() is not part of the Appender or Lifecycle interface, will it ever even be called? It's not called in the default start() implementation, and it's not overridden in any implementations.
These two are a little confusing. I recommend either JavaDoc clarification explaining the difference between the two, or simply removing close() (makes more sense to me).
Nick
Re: Appender close() vs stop()
Posted by Nick Williams <ni...@nicholaswilliams.net>.
Okay. Understood. Should I file a bug for someone to take care of that, or will someone just take care of that?
Note that an inner class within a *Test class @Overrides the close method but doesn't do anything within it and doesn't call it. A simple find usages will reveal it.
Nick
On Apr 27, 2013, at 9:35 AM, Gary Gregory wrote:
> I think it would be clearer to remove close in a district commit.
>
> Gary
>
> On Apr 27, 2013, at 10:12, Nick Williams <ni...@nicholaswilliams.net> wrote:
>
>> Ok. I'll remove it in my work on the database appenders.
>>
>> On Apr 27, 2013, at 8:56 AM, Ralph Goers wrote:
>>
>>> Sounds like I originally intended to use close but used stop instead. If close isn't being used then it can be removed.
>>>
>>> Sent from my iPad
>>>
>>> On Apr 27, 2013, at 4:46 AM, Nick Williams <ni...@nicholaswilliams.net> wrote:
>>>
>>>> While working on database appenders, I figured that the best place to "connect" is in Appender#start() and, likewise, the best place to "disconnect" is in Appender#stop(). However, then I noticed that AbstractAppender defines a close() method with the JavaDoc comment "Close the stream associated with the Appender." This could be read "Close the database connection associated with the Appender."
>>>>
>>>> So, with that said, where should I close the connection? In stop() or in close()? The AbstractOutputStreamAppender closes the stream in stop() (which is what I originally expected) and never overrides close(). Since close() is not part of the Appender or Lifecycle interface, will it ever even be called? It's not called in the default start() implementation, and it's not overridden in any implementations.
>>>>
>>>> These two are a little confusing. I recommend either JavaDoc clarification explaining the difference between the two, or simply removing close() (makes more sense to me).
>>>>
>>>> Nick
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
Re: Appender close() vs stop()
Posted by Nick Williams <ni...@nicholaswilliams.net>.
I have removed this in r1482810.
Nick
On Apr 27, 2013, at 10:57 AM, Ralph Goers wrote:
> Agreed, but since Nick doesn't have commit rights that is a bit more difficult for him. I suppose two patches could be supplied to the Jira for the database appender.
>
> Ralph
>
> On Apr 27, 2013, at 7:35 AM, Gary Gregory wrote:
>
>> I think it would be clearer to remove close in a district commit.
>>
>> Gary
>>
>> On Apr 27, 2013, at 10:12, Nick Williams <ni...@nicholaswilliams.net> wrote:
>>
>>> Ok. I'll remove it in my work on the database appenders.
>>>
>>> On Apr 27, 2013, at 8:56 AM, Ralph Goers wrote:
>>>
>>>> Sounds like I originally intended to use close but used stop instead. If close isn't being used then it can be removed.
>>>>
>>>> Sent from my iPad
>>>>
>>>> On Apr 27, 2013, at 4:46 AM, Nick Williams <ni...@nicholaswilliams.net> wrote:
>>>>
>>>>> While working on database appenders, I figured that the best place to "connect" is in Appender#start() and, likewise, the best place to "disconnect" is in Appender#stop(). However, then I noticed that AbstractAppender defines a close() method with the JavaDoc comment "Close the stream associated with the Appender." This could be read "Close the database connection associated with the Appender."
>>>>>
>>>>> So, with that said, where should I close the connection? In stop() or in close()? The AbstractOutputStreamAppender closes the stream in stop() (which is what I originally expected) and never overrides close(). Since close() is not part of the Appender or Lifecycle interface, will it ever even be called? It's not called in the default start() implementation, and it's not overridden in any implementations.
>>>>>
>>>>> These two are a little confusing. I recommend either JavaDoc clarification explaining the difference between the two, or simply removing close() (makes more sense to me).
>>>>>
>>>>> Nick
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org
Re: Appender close() vs stop()
Posted by Ralph Goers <ra...@dslextreme.com>.
Agreed, but since Nick doesn't have commit rights that is a bit more difficult for him. I suppose two patches could be supplied to the Jira for the database appender.
Ralph
On Apr 27, 2013, at 7:35 AM, Gary Gregory wrote:
> I think it would be clearer to remove close in a district commit.
>
> Gary
>
> On Apr 27, 2013, at 10:12, Nick Williams <ni...@nicholaswilliams.net> wrote:
>
>> Ok. I'll remove it in my work on the database appenders.
>>
>> On Apr 27, 2013, at 8:56 AM, Ralph Goers wrote:
>>
>>> Sounds like I originally intended to use close but used stop instead. If close isn't being used then it can be removed.
>>>
>>> Sent from my iPad
>>>
>>> On Apr 27, 2013, at 4:46 AM, Nick Williams <ni...@nicholaswilliams.net> wrote:
>>>
>>>> While working on database appenders, I figured that the best place to "connect" is in Appender#start() and, likewise, the best place to "disconnect" is in Appender#stop(). However, then I noticed that AbstractAppender defines a close() method with the JavaDoc comment "Close the stream associated with the Appender." This could be read "Close the database connection associated with the Appender."
>>>>
>>>> So, with that said, where should I close the connection? In stop() or in close()? The AbstractOutputStreamAppender closes the stream in stop() (which is what I originally expected) and never overrides close(). Since close() is not part of the Appender or Lifecycle interface, will it ever even be called? It's not called in the default start() implementation, and it's not overridden in any implementations.
>>>>
>>>> These two are a little confusing. I recommend either JavaDoc clarification explaining the difference between the two, or simply removing close() (makes more sense to me).
>>>>
>>>> Nick
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org
Re: Appender close() vs stop()
Posted by Gary Gregory <ga...@gmail.com>.
I think it would be clearer to remove close in a district commit.
Gary
On Apr 27, 2013, at 10:12, Nick Williams <ni...@nicholaswilliams.net> wrote:
> Ok. I'll remove it in my work on the database appenders.
>
> On Apr 27, 2013, at 8:56 AM, Ralph Goers wrote:
>
>> Sounds like I originally intended to use close but used stop instead. If close isn't being used then it can be removed.
>>
>> Sent from my iPad
>>
>> On Apr 27, 2013, at 4:46 AM, Nick Williams <ni...@nicholaswilliams.net> wrote:
>>
>>> While working on database appenders, I figured that the best place to "connect" is in Appender#start() and, likewise, the best place to "disconnect" is in Appender#stop(). However, then I noticed that AbstractAppender defines a close() method with the JavaDoc comment "Close the stream associated with the Appender." This could be read "Close the database connection associated with the Appender."
>>>
>>> So, with that said, where should I close the connection? In stop() or in close()? The AbstractOutputStreamAppender closes the stream in stop() (which is what I originally expected) and never overrides close(). Since close() is not part of the Appender or Lifecycle interface, will it ever even be called? It's not called in the default start() implementation, and it's not overridden in any implementations.
>>>
>>> These two are a little confusing. I recommend either JavaDoc clarification explaining the difference between the two, or simply removing close() (makes more sense to me).
>>>
>>> Nick
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org
Re: Appender close() vs stop()
Posted by Nick Williams <ni...@nicholaswilliams.net>.
Ok. I'll remove it in my work on the database appenders.
On Apr 27, 2013, at 8:56 AM, Ralph Goers wrote:
> Sounds like I originally intended to use close but used stop instead. If close isn't being used then it can be removed.
>
> Sent from my iPad
>
> On Apr 27, 2013, at 4:46 AM, Nick Williams <ni...@nicholaswilliams.net> wrote:
>
>> While working on database appenders, I figured that the best place to "connect" is in Appender#start() and, likewise, the best place to "disconnect" is in Appender#stop(). However, then I noticed that AbstractAppender defines a close() method with the JavaDoc comment "Close the stream associated with the Appender." This could be read "Close the database connection associated with the Appender."
>>
>> So, with that said, where should I close the connection? In stop() or in close()? The AbstractOutputStreamAppender closes the stream in stop() (which is what I originally expected) and never overrides close(). Since close() is not part of the Appender or Lifecycle interface, will it ever even be called? It's not called in the default start() implementation, and it's not overridden in any implementations.
>>
>> These two are a little confusing. I recommend either JavaDoc clarification explaining the difference between the two, or simply removing close() (makes more sense to me).
>>
>> Nick
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
Re: Appender close() vs stop()
Posted by Ralph Goers <rg...@apache.org>.
Sounds like I originally intended to use close but used stop instead. If close isn't being used then it can be removed.
Sent from my iPad
On Apr 27, 2013, at 4:46 AM, Nick Williams <ni...@nicholaswilliams.net> wrote:
> While working on database appenders, I figured that the best place to "connect" is in Appender#start() and, likewise, the best place to "disconnect" is in Appender#stop(). However, then I noticed that AbstractAppender defines a close() method with the JavaDoc comment "Close the stream associated with the Appender." This could be read "Close the database connection associated with the Appender."
>
> So, with that said, where should I close the connection? In stop() or in close()? The AbstractOutputStreamAppender closes the stream in stop() (which is what I originally expected) and never overrides close(). Since close() is not part of the Appender or Lifecycle interface, will it ever even be called? It's not called in the default start() implementation, and it's not overridden in any implementations.
>
> These two are a little confusing. I recommend either JavaDoc clarification explaining the difference between the two, or simply removing close() (makes more sense to me).
>
> Nick
---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org