logging-log4j-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ralph Goers <ralph.go...@dslextreme.com>
Subject Re: logging-log4j2 git commit: LOG4J2-124 - Add shutdown methods to LogManager
Date Sun, 24 Jan 2016 23:45:47 GMT
I only added all the methods to match the getContext() methods. If you guys want to eliminate
a few of them that is fine by me.

The protected methods match the protected getContext() methods.

I only implemented a single unit test because they are all essentially the same. All you end
up testing is getContext() again.  

Ralph

> On Jan 24, 2016, at 4:33 PM, Gary Gregory <garydgregory@gmail.com> wrote:
> 
> On Sun, Jan 24, 2016 at 2:43 PM, Remko Popma <remko.popma@gmail.com <mailto:remko.popma@gmail.com>>
wrote:
> Do we need all 6 methods?
> 
> I like the first three:
> * shutdown()
> * shutdown(boolean)
> * shutdown(LoggerContext)
> 
> I would argue that the next three are rare use cases. When needed these can be accomplished
by calling LogManager.shutdown(LogManager.getContext(some params)).
> 
> I would be in favor of removing these three convenience methods:
> * shutdown(String, boolean)
> * shutdown(String, ClassLoader, boolean)
> * shutdown(ClassLoader, boolean)
> 
> Thoughts? -Remko
> 
> That does not see bad to me. We have lots of getLogger() and getContext() methods. 
> 
> I wonder why these are protected through:
> 
> org.apache.logging.log4j.LogManager.shutdown(String, boolean)
> org.apache.logging.log4j.LogManager.shutdown(String, ClassLoader, boolean) 
> 
> Also, these along with:
> 
> org.apache.logging.log4j.LogManager.shutdown()
> org.apache.logging.log4j.LogManager.shutdown(boolean)
> org.apache.logging.log4j.LogManager.shutdown(ClassLoader, boolean)
> 
> have no senders, so no unit tests?
> 
> Gary
> 
> 
> 
> 
> On Mon, Jan 25, 2016 at 6:54 AM, Ralph Goers <ralph.goers@dslextreme.com <mailto:ralph.goers@dslextreme.com>>
wrote:
> Yes, Shutdownable is too weird. Closeable would imply there is a close method, not a
shutdown method.  I have my doubts about the try-with-resources use case here.  Virtually
all usages are probably going to be to disable automatic shutdown and then the user placing
the shutdown call somewhere they can control, which probably will have nothing to do with
initialization of logging. 
> 
> Ralph
> 
>> On Jan 24, 2016, at 2:43 PM, Gary Gregory <garydgregory@gmail.com <mailto:garydgregory@gmail.com>>
wrote:
>> 
>> Resending, got a error from my phone...
>> 
>> ---------- Forwarded message ----------
>> From: "Gary Gregory" <garydgregory@gmail.com <mailto:garydgregory@gmail.com>>
>> Date: Jan 24, 2016 1:41 PM
>> Subject: Re: logging-log4j2 git commit: LOG4J2-124 - Add shutdown methods to LogManager
>> To: <dev@logging.apache.org <mailto:dev@logging.apache.org>>
>> Cc: 
>> 
>> Hi all,
>> 
>> Any reason not use the usual -able postfix instead of ShutdownCapable: Shutdownable
sounds too weird? What about reusing plain old Closeable? That means you could use the context
in a try-with-resources block, a bonus.
>> 
>> Gary 
>> 
>> On Jan 24, 2016 10:18 AM, <rgoers@apache.org <mailto:rgoers@apache.org>>
wrote:
>> Repository: logging-log4j2
>> Updated Branches:
>>   refs/heads/master 7d3aac4b9 -> 2df3f0e72
>> 
>> 
>> LOG4J2-124 - Add shutdown methods to LogManager
>> 
>> 
>> Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo <http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo>
>> Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/2df3f0e7 <http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/2df3f0e7>
>> Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/2df3f0e7 <http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/2df3f0e7>
>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/2df3f0e7 <http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/2df3f0e7>
>> 
>> Branch: refs/heads/master
>> Commit: 2df3f0e7262c90e3fe1700f053eebf18491650d9
>> Parents: 7d3aac4
>> Author: Ralph Goers <rgoers@nextiva.com <mailto:rgoers@nextiva.com>>
>> Authored: Sun Jan 24 11:18:41 2016 -0700
>> Committer: Ralph Goers <rgoers@nextiva.com <mailto:rgoers@nextiva.com>>
>> Committed: Sun Jan 24 11:18:41 2016 -0700
>> 
>> ----------------------------------------------------------------------
>>  .../org/apache/logging/log4j/LogManager.java    | 62 ++++++++++++++++++++
>>  .../logging/log4j/spi/ShutdownCapable.java      | 17 ++++++
>>  .../apache/logging/log4j/LogManagerTest.java    |  7 +++
>>  .../logging/log4j/core/LoggerContext.java       |  8 ++-
>>  4 files changed, 93 insertions(+), 1 deletion(-)
>> ----------------------------------------------------------------------
>> 
>> 
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2df3f0e7/log4j-api/src/main/java/org/apache/logging/log4j/LogManager.java
<http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2df3f0e7/log4j-api/src/main/java/org/apache/logging/log4j/LogManager.java>
>> ----------------------------------------------------------------------
>> diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/LogManager.java b/log4j-api/src/main/java/org/apache/logging/log4j/LogManager.java
>> index f10e5a8..64c6ee5 100644
>> --- a/log4j-api/src/main/java/org/apache/logging/log4j/LogManager.java
>> +++ b/log4j-api/src/main/java/org/apache/logging/log4j/LogManager.java
>> @@ -27,6 +27,7 @@ import org.apache.logging.log4j.simple.SimpleLoggerContextFactory;
>>  import org.apache.logging.log4j.spi.LoggerContext;
>>  import org.apache.logging.log4j.spi.LoggerContextFactory;
>>  import org.apache.logging.log4j.spi.Provider;
>> +import org.apache.logging.log4j.spi.ShutdownCapable;
>>  import org.apache.logging.log4j.status.StatusLogger;
>>  import org.apache.logging.log4j.util.LoaderUtil;
>>  import org.apache.logging.log4j.util.PropertiesUtil;
>> @@ -285,6 +286,67 @@ public class LogManager {
>>      }
>> 
>>      /**
>> +     * Shutdown using the default LoggerContext.
>> +     * @since 2.6
>> +     */
>> +    public static void shutdown() {
>> +        shutdown(getContext());
>> +    }
>> +
>> +    /**
>> +     * Shutdown the logging system if the logging system supports it.
>> +     * @param currentContext if true the LoggerContext for the caller of this method
will be used.
>> +     * @since 2.6
>> +     */
>> +    public static void shutdown(boolean currentContext) {
>> +        shutdown(getContext(currentContext));
>> +    }
>> +
>> +    /**
>> +     * Shutdown the logging system if the logging system supports it.
>> +     * @param loader The ClassLoader for the context. If null the context will attempt
to determine the appropriate
>> +     *            ClassLoader.
>> +     * @param currentContext if false the LoggerContext appropriate for the caller
of this method will be used.
>> +     * @since 2.6
>> +     */
>> +    public static void shutdown(final ClassLoader loader, final boolean currentContext)
{
>> +        shutdown(getContext(loader, currentContext));
>> +    }
>> +
>> +    /**
>> +     * Shutdown the logging system if the logging system supports it.
>> +     * @param context the LoggerContext.
>> +     * @since 2.6
>> +     */
>> +    public static void shutdown(LoggerContext context) {
>> +        if (context != null && context instanceof ShutdownCapable) {
>> +            ((ShutdownCapable) context).shutdown();
>> +        }
>> +    }
>> +
>> +    /**
>> +     * Shutdown the logging system if the logging system supports it.
>> +     * @param fqcn The fully qualified class name of the Class that this method
is a member of.
>> +     * @param currentContext if false the LoggerContext appropriate for the caller
of this method will be used.
>> +     * @since 2.6
>> +     */
>> +    protected static void shutdown(final String fqcn, final boolean currentContext)
{
>> +        shutdown(getContext(fqcn, currentContext));
>> +    }
>> +
>> +    /**
>> +     * Shutdown the logging system if the logging system supports it.
>> +     * @param fqcn The fully qualified class name of the Class that this method
is a member of.
>> +     * @param loader The ClassLoader for the context. If null the context will attempt
to determine the appropriate
>> +     *            ClassLoader.
>> +     * @param currentContext if false the LoggerContext appropriate for the caller
of this method will be used.
>> +     * @since 2.6
>> +     */
>> +    protected static void shutdown(final String fqcn, final ClassLoader loader,
final boolean currentContext) {
>> +        shutdown(getContext(fqcn, loader, currentContext));
>> +    }
>> +
>> +    /**
>>       * Returns the current LoggerContextFactory.
>>       *
>>       * @return The LoggerContextFactory.
>> 
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2df3f0e7/log4j-api/src/main/java/org/apache/logging/log4j/spi/ShutdownCapable.java
<http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2df3f0e7/log4j-api/src/main/java/org/apache/logging/log4j/spi/ShutdownCapable.java>
>> ----------------------------------------------------------------------
>> diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/spi/ShutdownCapable.java
b/log4j-api/src/main/java/org/apache/logging/log4j/spi/ShutdownCapable.java
>> new file mode 100644
>> index 0000000..a46ef60
>> --- /dev/null
>> +++ b/log4j-api/src/main/java/org/apache/logging/log4j/spi/ShutdownCapable.java
>> @@ -0,0 +1,17 @@
>> +/*
>> + * Copyright (c) 2016 Nextiva, Inc. to Present.
>> + * All rights reserved.
>> + */
>> +package org.apache.logging.log4j.spi;
>> +
>> +/**
>> + * Interface to be implemented by LoggerContext's that provide a shutdown method.
>> + * @since 2.6
>> + */
>> +public interface ShutdownCapable {
>> +
>> +    /**
>> +     * Requests that the logging implementation shut down.
>> +     */
>> +    void shutdown();
>> +}
>> 
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2df3f0e7/log4j-api/src/test/java/org/apache/logging/log4j/LogManagerTest.java
<http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2df3f0e7/log4j-api/src/test/java/org/apache/logging/log4j/LogManagerTest.java>
>> ----------------------------------------------------------------------
>> diff --git a/log4j-api/src/test/java/org/apache/logging/log4j/LogManagerTest.java
b/log4j-api/src/test/java/org/apache/logging/log4j/LogManagerTest.java
>> index 48f0eea..596a9f2 100644
>> --- a/log4j-api/src/test/java/org/apache/logging/log4j/LogManagerTest.java
>> +++ b/log4j-api/src/test/java/org/apache/logging/log4j/LogManagerTest.java
>> @@ -17,6 +17,7 @@
>>  package org.apache.logging.log4j;
>> 
>>  import org.apache.logging.log4j.message.ParameterizedMessageFactory;
>> +import org.apache.logging.log4j.spi.LoggerContext;
>>  import org.junit.Test;
>> 
>>  import static org.junit.Assert.*;
>> @@ -53,4 +54,10 @@ public class LogManagerTest {
>>          assertNotNull("No Logger returned", logger);
>>          assertTrue("Incorrect Logger name: " + logger.getName(),LogManagerTest.class.getName().equals(logger.getName()));
>>      }
>> +
>> +    @Test
>> +    public void testShutdown() {
>> +        LoggerContext loggerContext = LogManager.getContext(false);
>> +        LogManager.shutdown(loggerContext);
>> +    }
>>  }
>> 
>> http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2df3f0e7/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
<http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/2df3f0e7/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java>
>> ----------------------------------------------------------------------
>> diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
>> index 42efbb5..fcdfc16 100644
>> --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
>> +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
>> @@ -45,6 +45,7 @@ import org.apache.logging.log4j.message.MessageFactory;
>>  import org.apache.logging.log4j.spi.AbstractLogger;
>>  import org.apache.logging.log4j.spi.LoggerContextFactory;
>>  import org.apache.logging.log4j.spi.LoggerContextKey;
>> +import org.apache.logging.log4j.spi.ShutdownCapable;
>> 
>>  import static org.apache.logging.log4j.core.util.ShutdownCallbackRegistry.*;
>> 
>> @@ -54,7 +55,7 @@ import static org.apache.logging.log4j.core.util.ShutdownCallbackRegistry.*;
>>   * filters, etc and will be atomically updated whenever a reconfigure occurs.
>>   */
>>  public class LoggerContext extends AbstractLifeCycle implements org.apache.logging.log4j.spi.LoggerContext,
>> -        ConfigurationListener {
>> +        ShutdownCapable, ConfigurationListener {
>> 
>>      /**
>>       * Property name of the property change event fired if the configuration is
changed.
>> @@ -278,6 +279,11 @@ public class LoggerContext extends AbstractLifeCycle implements
org.apache.loggi
>>      }
>> 
>>      @Override
>> +    public void shutdown() {
>> +        stop();
>> +    }
>> +
>> +    @Override
>>      public void stop() {
>>          LOGGER.debug("Stopping LoggerContext[name={}, {}]...", getName(), this);
>>          configLock.lock();
>> 
> 
> 
> 
> 
> 
> -- 
> E-Mail: garydgregory@gmail.com <mailto:garydgregory@gmail.com> | ggregory@apache.orgĀ 
<mailto:ggregory@apache.org>
> Java Persistence with Hibernate, Second Edition <http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com <http://garygregory.wordpress.com/> 
> Home: http://garygregory.com/ <http://garygregory.com/>
> Tweet! http://twitter.com/GaryGregory <http://twitter.com/GaryGregory>

Mime
View raw message