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


On Mon, Jan 25, 2016 at 6:54 AM, Ralph Goers <[email protected]>
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 <[email protected]> wrote:
>
> Resending, got a error from my phone...
> ---------- Forwarded message ----------
> From: "Gary Gregory" <[email protected]>
> Date: Jan 24, 2016 1:41 PM
> Subject: Re: logging-log4j2 git commit: LOG4J2-124 - Add shutdown methods
> to LogManager
> To: <[email protected]>
> 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, <[email protected]> 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
>> Commit:
>> 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
>> Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/2df3f0e7
>>
>> Branch: refs/heads/master
>> Commit: 2df3f0e7262c90e3fe1700f053eebf18491650d9
>> Parents: 7d3aac4
>> Author: Ralph Goers <[email protected]>
>> Authored: Sun Jan 24 11:18:41 2016 -0700
>> Committer: Ralph Goers <[email protected]>
>> 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
>> ----------------------------------------------------------------------
>> 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
>> ----------------------------------------------------------------------
>> 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
>> ----------------------------------------------------------------------
>> 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
>> ----------------------------------------------------------------------
>> 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();
>>
>>
>

Reply via email to