You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by fluxroot <gi...@git.apache.org> on 2018/06/12 20:35:58 UTC

[GitHub] logging-log4j2 issue #176: [LOG4J2-1721] Allow composite configuration for c...

Github user fluxroot commented on the issue:

    https://github.com/apache/logging-log4j2/pull/176
  
    That was my initial thought as well. However, there have been a couple of issues which made me copy the functionality from Configurator.initialize() over to Log4jWebInitializerImpl.getConfigURIs():
    
    1. log4j-web is an optional dependency and only useful in a servlet container environment. Common code would have to go into log4j-core.
    2. Parsing config location in Log4jWebInitializerImpl.getConfigURI() is more complicated. In addition to resolving file URIs, we have to search WEB-INF and also consider servlet context name to find the matching config.
    
    The solution in Java 8 would be to pass a closure to resolve the config location. For Java 7 I guess there's no elegant solution for this problem (or maybe I'm missing something :)). So for now I would leave this fix as is.


---

Re: [GitHub] logging-log4j2 issue #176: [LOG4J2-1721] Allow composite configuration for c...

Posted by Phokham Nonava <pn...@nonava.com>.
Hi there,

Is there anything I can do to move this issue forward? As I understand 
Matt Sicker has already reviewed and approved the PR on GitHub.

Thanks,
Poke

On 13.06.18 17:17, Phokham Nonava wrote:
> Right, for the release-2.x branch I would leave the solution as it is. 
> For the master branch I could create another pull request though. Would 
> it be ok if we could merge this one and I'll create another one for the 
> master branch?
> 
> On 13.06.18 14:37, Matt Sicker wrote:
>> The master branch is Java 8, and the release-2.x branch is Java 7. We 
>> have
>> some functional interfaces available.
>>
>> On Tue, Jun 12, 2018 at 16:35, fluxroot <gi...@git.apache.org> wrote:
>>
>>> Github user fluxroot commented on the issue:
>>>
>>>      https://github.com/apache/logging-log4j2/pull/176
>>>
>>>      That was my initial thought as well. However, there have been a 
>>> couple
>>> of issues which made me copy the functionality from
>>> Configurator.initialize() over to 
>>> Log4jWebInitializerImpl.getConfigURIs():
>>>
>>>      1. log4j-web is an optional dependency and only useful in a servlet
>>> container environment. Common code would have to go into log4j-core.
>>>      2. Parsing config location in 
>>> Log4jWebInitializerImpl.getConfigURI()
>>> is more complicated. In addition to resolving file URIs, we have to 
>>> search
>>> WEB-INF and also consider servlet context name to find the matching 
>>> config.
>>>
>>>      The solution in Java 8 would be to pass a closure to resolve the
>>> config location. For Java 7 I guess there's no elegant solution for this
>>> problem (or maybe I'm missing something :)). So for now I would leave 
>>> this
>>> fix as is.

Re: [GitHub] logging-log4j2 issue #176: [LOG4J2-1721] Allow composite configuration for c...

Posted by Phokham Nonava <pn...@nonava.com>.
Right, for the release-2.x branch I would leave the solution as it is. 
For the master branch I could create another pull request though. Would 
it be ok if we could merge this one and I'll create another one for the 
master branch?

On 13.06.18 14:37, Matt Sicker wrote:
> The master branch is Java 8, and the release-2.x branch is Java 7. We have
> some functional interfaces available.
> 
> On Tue, Jun 12, 2018 at 16:35, fluxroot <gi...@git.apache.org> wrote:
> 
>> Github user fluxroot commented on the issue:
>>
>>      https://github.com/apache/logging-log4j2/pull/176
>>
>>      That was my initial thought as well. However, there have been a couple
>> of issues which made me copy the functionality from
>> Configurator.initialize() over to Log4jWebInitializerImpl.getConfigURIs():
>>
>>      1. log4j-web is an optional dependency and only useful in a servlet
>> container environment. Common code would have to go into log4j-core.
>>      2. Parsing config location in Log4jWebInitializerImpl.getConfigURI()
>> is more complicated. In addition to resolving file URIs, we have to search
>> WEB-INF and also consider servlet context name to find the matching config.
>>
>>      The solution in Java 8 would be to pass a closure to resolve the
>> config location. For Java 7 I guess there's no elegant solution for this
>> problem (or maybe I'm missing something :)). So for now I would leave this
>> fix as is.

Re: [GitHub] logging-log4j2 issue #176: [LOG4J2-1721] Allow composite configuration for c...

Posted by Matt Sicker <bo...@gmail.com>.
The master branch is Java 8, and the release-2.x branch is Java 7. We have
some functional interfaces available.

On Tue, Jun 12, 2018 at 16:35, fluxroot <gi...@git.apache.org> wrote:

> Github user fluxroot commented on the issue:
>
>     https://github.com/apache/logging-log4j2/pull/176
>
>     That was my initial thought as well. However, there have been a couple
> of issues which made me copy the functionality from
> Configurator.initialize() over to Log4jWebInitializerImpl.getConfigURIs():
>
>     1. log4j-web is an optional dependency and only useful in a servlet
> container environment. Common code would have to go into log4j-core.
>     2. Parsing config location in Log4jWebInitializerImpl.getConfigURI()
> is more complicated. In addition to resolving file URIs, we have to search
> WEB-INF and also consider servlet context name to find the matching config.
>
>     The solution in Java 8 would be to pass a closure to resolve the
> config location. For Java 7 I guess there's no elegant solution for this
> problem (or maybe I'm missing something :)). So for now I would leave this
> fix as is.
>
>
> ---
>
-- 
Matt Sicker <bo...@gmail.com>