IgniteLogger.isQuiet() inconsistency

[Artem Shutak]

Hi, Igniters,

In process of investigation of a user request "Disable ignite console logs"
(
http://apache-ignite-users.70518.x6.nabble.com/Disable-ignite-console-logs-td310.html#a330),
I've found inconsistency at implementations of IgniteLogger.isQuiet():
- Javadoc of the method says:
    /**
     * Tests whether {@code info} and {@code debug} levels are turned off.
     *
     * @return Whether {@code info} and {@code debug} levels are turned off.
     */
- IgniteJclLogger.isQuiet() implementation corresponds to javadoc.
- isQuiet() implementations for Log4JLogger and JavaLogger don't correspond
to javadoc and "quite" option related to IGNITE_QUITE system variable.

For me, the implementation of Log4JLogger is better and we should change
javadoc and change implementation of IgniteJclLogger accordingly. I've
filed a bug on it: https://issues.apache.org/jira/browse/IGNITE-923.

Any objections?

-- Artem --

[Gianfranco Murador]

Hi Artem,
  I've changed the implementation for the module  ( IGNITE-788 branch ) in
according with the IgniteJcl implementation. I think that  IGNITE_QUITE sys
variabile should not be  used in the addConsoleAppenderIfNeeded , as well.

replace this
boolean quiet = Boolean.valueOf(System.getProperty(IGNITE_QUIET,
                    "true"));

with

boolean quiet = isQuite();

In additions, I'd like to ask you some hints about the new implementations
of the IgniteLogger with log4j2.
We need to insert some informations about the currente nodeId, I'd like to
implement this feature by configuration in this way:

    <Appender type="File" name="File"
fileName="C:/work/log/ignite-${sys:nodeid}.log">
When nodeid is a System properties setted in thi way:

 System.setProperty("nodeid", ignite.cluster().localNode().id());

Let me know any suggestions.
Thank you, Gianfranco





2015-05-20 14:18 GMT+02:00 Artiom Shutak <[hidden email]>:

> Hi, Igniters,
>
> In process of investigation of a user request "Disable ignite console logs"
> (
>
> http://apache-ignite-users.70518.x6.nabble.com/Disable-ignite-console-logs-td310.html#a330
> ),
> I've found inconsistency at implementations of IgniteLogger.isQuiet():
> - Javadoc of the method says:
>     /**
>      * Tests whether {@code info} and {@code debug} levels are turned off.
>      *
>      * @return Whether {@code info} and {@code debug} levels are turned
> off.
>      */
> - IgniteJclLogger.isQuiet() implementation corresponds to javadoc.
> - isQuiet() implementations for Log4JLogger and JavaLogger don't correspond
> to javadoc and "quite" option related to IGNITE_QUITE system variable.
>
> For me, the implementation of Log4JLogger is better and we should change
> javadoc and change implementation of IgniteJclLogger accordingly. I've
> filed a bug on it: https://issues.apache.org/jira/browse/IGNITE-923.
>
> Any objections?
>
> -- Artem --
>

[Artem Shutak]

Hi Gianfranco,

I don't understand exactly what we are talking about :)

If we are about https://issues.apache.org/jira/browse/IGNITE-923, then
- It has to be fixed in a separate thread (don't do it together with
IGNITE-788).
- If you have any objections about the way to fix IGNITE-923. Can you
describe your way with pluses and minuses? It would be great, if we will
talk about it with some examples (use cases). You can do it in comments to
IGNITE-923.

If we are about https://issues.apache.org/jira/browse/IGNITE-788, then
- The main suggestion is to use ignite-log4j implementation as gold source
for your implementation.
- If you want to add some functionality, that there is no at ignite-log4j,
then we should to discuss it at new thread and it can be done in bounds of
new task (maybe there is another way to do it (already), or we should to
add this functionality for all loggers).

Thanks,
Artem.

On Sat, May 23, 2015 at 2:04 PM, Gianfranco Murador <
[hidden email]> wrote:

> Hi Artem,
>   I've changed the implementation for the module  ( IGNITE-788 branch ) in
> according with the IgniteJcl implementation. I think that  IGNITE_QUITE sys
> variabile should not be  used in the addConsoleAppenderIfNeeded , as well.
>
> replace this
> boolean quiet = Boolean.valueOf(System.getProperty(IGNITE_QUIET,
>                     "true"));
>
> with
>
> boolean quiet = isQuite();
>
> In additions, I'd like to ask you some hints about the new implementations
> of the IgniteLogger with log4j2.
> We need to insert some informations about the currente nodeId, I'd like to
> implement this feature by configuration in this way:
>
>     <Appender type="File" name="File"
> fileName="C:/work/log/ignite-${sys:nodeid}.log">
> When nodeid is a System properties setted in thi way:
>
>  System.setProperty("nodeid", ignite.cluster().localNode().id());
>
> Let me know any suggestions.
> Thank you, Gianfranco
>
>
>
>
>
> 2015-05-20 14:18 GMT+02:00 Artiom Shutak <[hidden email]>:
>
> > Hi, Igniters,
> >
> > In process of investigation of a user request "Disable ignite console
> logs"
> > (
> >
> >
> http://apache-ignite-users.70518.x6.nabble.com/Disable-ignite-console-logs-td310.html#a330
> > ),
> > I've found inconsistency at implementations of IgniteLogger.isQuiet():
> > - Javadoc of the method says:
> >     /**
> >      * Tests whether {@code info} and {@code debug} levels are turned
> off.
> >      *
> >      * @return Whether {@code info} and {@code debug} levels are turned
> > off.
> >      */
> > - IgniteJclLogger.isQuiet() implementation corresponds to javadoc.
> > - isQuiet() implementations for Log4JLogger and JavaLogger don't
> correspond
> > to javadoc and "quite" option related to IGNITE_QUITE system variable.
> >
> > For me, the implementation of Log4JLogger is better and we should change
> > javadoc and change implementation of IgniteJclLogger accordingly. I've
> > filed a bug on it: https://issues.apache.org/jira/browse/IGNITE-923.
> >
> > Any objections?
> >
> > -- Artem --
> >
>

[Gianfranco Murador]

Hi Artiom,
 The method implementation ( isQuite() ) is in ingnite-log4j, so I'd like
to fix the implementation in the same branch, that's it.
Regards,
 Gianfranco

2015-05-26 11:43 GMT+02:00 Artiom Shutak <[hidden email]>:

> Hi Gianfranco,
>
> I don't understand exactly what we are talking about :)
>
> If we are about https://issues.apache.org/jira/browse/IGNITE-923, then
> - It has to be fixed in a separate thread (don't do it together with
> IGNITE-788).
> - If you have any objections about the way to fix IGNITE-923. Can you
> describe your way with pluses and minuses? It would be great, if we will
> talk about it with some examples (use cases). You can do it in comments to
> IGNITE-923.
>
> If we are about https://issues.apache.org/jira/browse/IGNITE-788, then
> - The main suggestion is to use ignite-log4j implementation as gold source
> for your implementation.
> - If you want to add some functionality, that there is no at ignite-log4j,
> then we should to discuss it at new thread and it can be done in bounds of
> new task (maybe there is another way to do it (already), or we should to
> add this functionality for all loggers).
>
> Thanks,
> Artem.
>
> On Sat, May 23, 2015 at 2:04 PM, Gianfranco Murador <
> [hidden email]> wrote:
>
> > Hi Artem,
> >   I've changed the implementation for the module  ( IGNITE-788 branch )
> in
> > according with the IgniteJcl implementation. I think that  IGNITE_QUITE
> sys
> > variabile should not be  used in the addConsoleAppenderIfNeeded , as
> well.
> >
> > replace this
> > boolean quiet = Boolean.valueOf(System.getProperty(IGNITE_QUIET,
> >                     "true"));
> >
> > with
> >
> > boolean quiet = isQuite();
> >
> > In additions, I'd like to ask you some hints about the new
> implementations
> > of the IgniteLogger with log4j2.
> > We need to insert some informations about the currente nodeId, I'd like
> to
> > implement this feature by configuration in this way:
> >
> >     <Appender type="File" name="File"
> > fileName="C:/work/log/ignite-${sys:nodeid}.log">
> > When nodeid is a System properties setted in thi way:
> >
> >  System.setProperty("nodeid", ignite.cluster().localNode().id());
> >
> > Let me know any suggestions.
> > Thank you, Gianfranco
> >
> >
> >
> >
> >
> > 2015-05-20 14:18 GMT+02:00 Artiom Shutak <[hidden email]>:
> >
> > > Hi, Igniters,
> > >
> > > In process of investigation of a user request "Disable ignite console
> > logs"
> > > (
> > >
> > >
> >
> http://apache-ignite-users.70518.x6.nabble.com/Disable-ignite-console-logs-td310.html#a330
> > > ),
> > > I've found inconsistency at implementations of IgniteLogger.isQuiet():
> > > - Javadoc of the method says:
> > >     /**
> > >      * Tests whether {@code info} and {@code debug} levels are turned
> > off.
> > >      *
> > >      * @return Whether {@code info} and {@code debug} levels are turned
> > > off.
> > >      */
> > > - IgniteJclLogger.isQuiet() implementation corresponds to javadoc.
> > > - isQuiet() implementations for Log4JLogger and JavaLogger don't
> > correspond
> > > to javadoc and "quite" option related to IGNITE_QUITE system variable.
> > >
> > > For me, the implementation of Log4JLogger is better and we should
> change
> > > javadoc and change implementation of IgniteJclLogger accordingly. I've
> > > filed a bug on it: https://issues.apache.org/jira/browse/IGNITE-923.
> > >
> > > Any objections?
> > >
> > > -- Artem --
> > >
> >
>

[Gianfranco Murador]

I fix it in a separate branch, I'd like to discuss the other JIRA (IGNITE
788 ) implementation details. I have finished the implementation and I
commit a patchi in the JIRA as soon as possible.
Thanks, Gianfranco

2015-05-26 17:35 GMT+02:00 Gianfranco Murador <[hidden email]>
:

> Hi Artiom,
>  The method implementation ( isQuite() ) is in ingnite-log4j, so I'd like
> to fix the implementation in the same branch, that's it.
> Regards,
>  Gianfranco
>
> 2015-05-26 11:43 GMT+02:00 Artiom Shutak <[hidden email]>:
>
>> Hi Gianfranco,
>>
>> I don't understand exactly what we are talking about :)
>>
>> If we are about https://issues.apache.org/jira/browse/IGNITE-923, then
>> - It has to be fixed in a separate thread (don't do it together with
>> IGNITE-788).
>> - If you have any objections about the way to fix IGNITE-923. Can you
>> describe your way with pluses and minuses? It would be great, if we will
>> talk about it with some examples (use cases). You can do it in comments to
>> IGNITE-923.
>>
>> If we are about https://issues.apache.org/jira/browse/IGNITE-788, then
>> - The main suggestion is to use ignite-log4j implementation as gold source
>> for your implementation.
>> - If you want to add some functionality, that there is no at ignite-log4j,
>> then we should to discuss it at new thread and it can be done in bounds of
>> new task (maybe there is another way to do it (already), or we should to
>> add this functionality for all loggers).
>>
>> Thanks,
>> Artem.
>>
>> On Sat, May 23, 2015 at 2:04 PM, Gianfranco Murador <
>> [hidden email]> wrote:
>>
>> > Hi Artem,
>> >   I've changed the implementation for the module  ( IGNITE-788 branch )
>> in
>> > according with the IgniteJcl implementation. I think that  IGNITE_QUITE
>> sys
>> > variabile should not be  used in the addConsoleAppenderIfNeeded , as
>> well.
>> >
>> > replace this
>> > boolean quiet = Boolean.valueOf(System.getProperty(IGNITE_QUIET,
>> >                     "true"));
>> >
>> > with
>> >
>> > boolean quiet = isQuite();
>> >
>> > In additions, I'd like to ask you some hints about the new
>> implementations
>> > of the IgniteLogger with log4j2.
>> > We need to insert some informations about the currente nodeId, I'd like
>> to
>> > implement this feature by configuration in this way:
>> >
>> >     <Appender type="File" name="File"
>> > fileName="C:/work/log/ignite-${sys:nodeid}.log">
>> > When nodeid is a System properties setted in thi way:
>> >
>> >  System.setProperty("nodeid", ignite.cluster().localNode().id());
>> >
>> > Let me know any suggestions.
>> > Thank you, Gianfranco
>> >
>> >
>> >
>> >
>> >
>> > 2015-05-20 14:18 GMT+02:00 Artiom Shutak <[hidden email]>:
>> >
>> > > Hi, Igniters,
>> > >
>> > > In process of investigation of a user request "Disable ignite console
>> > logs"
>> > > (
>> > >
>> > >
>> >
>> http://apache-ignite-users.70518.x6.nabble.com/Disable-ignite-console-logs-td310.html#a330
>> > > ),
>> > > I've found inconsistency at implementations of IgniteLogger.isQuiet():
>> > > - Javadoc of the method says:
>> > >     /**
>> > >      * Tests whether {@code info} and {@code debug} levels are turned
>> > off.
>> > >      *
>> > >      * @return Whether {@code info} and {@code debug} levels are
>> turned
>> > > off.
>> > >      */
>> > > - IgniteJclLogger.isQuiet() implementation corresponds to javadoc.
>> > > - isQuiet() implementations for Log4JLogger and JavaLogger don't
>> > correspond
>> > > to javadoc and "quite" option related to IGNITE_QUITE system variable.
>> > >
>> > > For me, the implementation of Log4JLogger is better and we should
>> change
>> > > javadoc and change implementation of IgniteJclLogger accordingly. I've
>> > > filed a bug on it: https://issues.apache.org/jira/browse/IGNITE-923.
>> > >
>> > > Any objections?
>> > >
>> > > -- Artem --
>> > >
>> >
>>
>
>

[dsetrakyan]

On Tue, May 26, 2015 at 8:40 AM, Gianfranco Murador <
[hidden email]> wrote:

> I fix it in a separate branch, I'd like to discuss the other JIRA (IGNITE
> 788 ) implementation details. I have finished the implementation and I
> commit a patchi in the JIRA as soon as possible.
>

Thanks, Gianfranco! Looking forward to it.