Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Please find enclosed a patch (diff file) and a log4j.properties file.

      You should create a src/etc directory for main and core and place the properties file there.

      Cheers,
      Nick

      1. logging_patch.txt
        11 kB
        Nick Faiz
      2. log4j.properties
        0.2 kB
        Nick Faiz
      3. log_patch2.txt
        12 kB
        Nick Faiz
      4. logging_patch_3.txt
        11 kB
        Nick Faiz

        Activity

        Hide
        Nick Faiz added a comment -

        Modifications include adding a preGoal for the 'jar:jar' goal to
        the maven.xml of each project to ensure that the log4j.properties is
        within the build jar. This preGoal assumes that the log4j.properties
        file can be found in main/src/etc/ and core/src/etc/ .

        Show
        Nick Faiz added a comment - Modifications include adding a preGoal for the 'jar:jar' goal to the maven.xml of each project to ensure that the log4j.properties is within the build jar. This preGoal assumes that the log4j.properties file can be found in main/src/etc/ and core/src/etc/ .
        Hide
        Emmanuel Lecharny added a comment -

        Hi, Nick,

        could you modify your patch a little bit? The Category class is deprecated (http://www.docjar.com/docs/api/org/apache/log4j/Category.html)
        Logger is the class to use.

        Is this also possible that you add a test before eacjh log.debug call ?

        log.debug( "Search attempt using filter '" + filter + "' "
        + "with scope '" + scope + "' and a return limit of '" + limit
        + "'" );

        is expensive as the string is built even if we are not in debug mode.

        It's better to write :

        if (log.isDebugEnabled())

        { log.debug(...) }

        Even better, add a
        private static final boolean DEBUG = log.isDebugEnabled();

        and use it this way :

        if (DEBUG)

        { log.debug(...) }

        As DEBUG will be evaluated only once, the test is faster (no function called).

        Thanks a lot !

        Show
        Emmanuel Lecharny added a comment - Hi, Nick, could you modify your patch a little bit? The Category class is deprecated ( http://www.docjar.com/docs/api/org/apache/log4j/Category.html ) Logger is the class to use. Is this also possible that you add a test before eacjh log.debug call ? log.debug( "Search attempt using filter '" + filter + "' " + "with scope '" + scope + "' and a return limit of '" + limit + "'" ); is expensive as the string is built even if we are not in debug mode. It's better to write : if (log.isDebugEnabled()) { log.debug(...) } Even better, add a private static final boolean DEBUG = log.isDebugEnabled(); and use it this way : if (DEBUG) { log.debug(...) } As DEBUG will be evaluated only once, the test is faster (no function called). Thanks a lot !
        Hide
        Nick Faiz added a comment -

        Hi Emmanuel,
        Good idea! Thanks for making me not be lazy.

        There seems to be a logical inconsistency between the MainFrame class, which uses FilterDialog's own logging settings and the standard log4j logging levels.

        After applying the changes you recommended we run into a scenario where the MainFrame is dumping some of its own logging information, depending on how a setting in a GUI somewhere (I think).

        Anyway, see line 531 of MainFrame.class. This is what I ended up doing:

        else if ( mode == FilterDialog.DEBUG_MODE )
        {
        if (DEBUG)

        { log.debug( "Search attempt using filter '" + dialog.getFilter() + "' " + "with scope '" + dialog.getScope() + "' and a return limit of '" + dialog.getLimit() + "'" ); }

        else //it would seem we need to make a debug statement, based on a program instruction

        { //which differs to the log4j priority. //in this circumstance we simply use the old System printwriter for stdout. System.out.println( "Search attempt using filter '" + dialog.getFilter() + "' " + "with scope '" + dialog.getScope() + "' and a return limit of '" + dialog.getLimit() + "'" ); }

        }

        I guess Id ask the question - should the FilterDialog decide anything about logging levels?

        Cheers,
        Nick

        Show
        Nick Faiz added a comment - Hi Emmanuel, Good idea! Thanks for making me not be lazy. There seems to be a logical inconsistency between the MainFrame class, which uses FilterDialog's own logging settings and the standard log4j logging levels. After applying the changes you recommended we run into a scenario where the MainFrame is dumping some of its own logging information, depending on how a setting in a GUI somewhere (I think). Anyway, see line 531 of MainFrame.class. This is what I ended up doing: else if ( mode == FilterDialog.DEBUG_MODE ) { if (DEBUG) { log.debug( "Search attempt using filter '" + dialog.getFilter() + "' " + "with scope '" + dialog.getScope() + "' and a return limit of '" + dialog.getLimit() + "'" ); } else //it would seem we need to make a debug statement, based on a program instruction { //which differs to the log4j priority. //in this circumstance we simply use the old System printwriter for stdout. System.out.println( "Search attempt using filter '" + dialog.getFilter() + "' " + "with scope '" + dialog.getScope() + "' and a return limit of '" + dialog.getLimit() + "'" ); } } I guess Id ask the question - should the FilterDialog decide anything about logging levels? Cheers, Nick
        Hide
        Emmanuel Lecharny added a comment -

        Hi Nick,

        in MainFrame.class, I think that keeping the code this way should be enough :

        ... else if ( mode == FilterDialog.DEBUG_MODE )

        { log.debug( "Search attempt using filter '" + dialog.getFilter() + "' " + "with scope '" + dialog.getScope() + "' and a return limit of '" + dialog.getLimit() + "'" ); }

        You already have a test on the level, so do not care about the string being built.

        Anyway, yes, I think there is an inconstency about debug level used in log4j and FilterDialog.

        Be aware that the little optimization I suggested is better suited for server side, not for GUI. So use it when you think it fits your need. Personnaly, I found it confortable to use it everywhere, because it don't arm (except when you want to switch to debug dynamically).

        About your last question, I don't know. This is up to you to decide if it should !

        Keep up the good job ! We need guys like you working on those pieces of code !

        Show
        Emmanuel Lecharny added a comment - Hi Nick, in MainFrame.class, I think that keeping the code this way should be enough : ... else if ( mode == FilterDialog.DEBUG_MODE ) { log.debug( "Search attempt using filter '" + dialog.getFilter() + "' " + "with scope '" + dialog.getScope() + "' and a return limit of '" + dialog.getLimit() + "'" ); } You already have a test on the level, so do not care about the string being built. Anyway, yes, I think there is an inconstency about debug level used in log4j and FilterDialog. Be aware that the little optimization I suggested is better suited for server side, not for GUI. So use it when you think it fits your need. Personnaly, I found it confortable to use it everywhere, because it don't arm (except when you want to switch to debug dynamically). About your last question, I don't know. This is up to you to decide if it should ! Keep up the good job ! We need guys like you working on those pieces of code !
        Hide
        Alex Karasulu added a comment -

        Why use log4j 1.2.XXX rather than the 1.3? Just curious.

        Show
        Alex Karasulu added a comment - Why use log4j 1.2.XXX rather than the 1.3? Just curious.
        Hide
        Nick Faiz added a comment -

        Hi Alex,
        Just for convenience. I know that it works. I only discovered just now that a 1.2.9 was available at ibiblio- http://www.ibiblio.org/maven/log4j/jars/ . If 1.3 is the latest it'd be the one to try.

        I'll read through http://www.qos.ch/logging/sc.jsp , r.e. Repository Selectors, on the weekend.

        Cheers,
        Nick

        Show
        Nick Faiz added a comment - Hi Alex, Just for convenience. I know that it works. I only discovered just now that a 1.2.9 was available at ibiblio- http://www.ibiblio.org/maven/log4j/jars/ . If 1.3 is the latest it'd be the one to try. I'll read through http://www.qos.ch/logging/sc.jsp , r.e. Repository Selectors, on the weekend. Cheers, Nick
        Hide
        Emmanuel Lecharny added a comment -

        Hi guys,

        The current version is 1.2.11. The next one will be 1.3, and is expected to be out on October.

        Show
        Emmanuel Lecharny added a comment - Hi guys, The current version is 1.2.11. The next one will be 1.3, and is expected to be out on October.
        Hide
        Nick Faiz added a comment -

        Please find attached a configuration for nlog4j, relying on the org.slf4j.Logger interface.

        The logging configuration relies upon a log4j.properties file being present in main/src/etc (a directory which should be created). The project.xml states that main/src/etc/log4j.properties is a resource for the source build (handy for including stuff on the classpath in IDEA, probably Eclipse too).

        The apacheds project.xml now expresses this dependency - so main, shared, and core will pick it up too.

        <dependency>
        <groupId>org.slf4j</groupId>
        <artifactId>nlog4j</artifactId>
        <version>1.2.14</version>
        <url>http://slf4j.org/nlog4j</url>
        </dependency>

        Cheers all,
        Nick

        Show
        Nick Faiz added a comment - Please find attached a configuration for nlog4j, relying on the org.slf4j.Logger interface. The logging configuration relies upon a log4j.properties file being present in main/src/etc (a directory which should be created). The project.xml states that main/src/etc/log4j.properties is a resource for the source build (handy for including stuff on the classpath in IDEA, probably Eclipse too). The apacheds project.xml now expresses this dependency - so main, shared, and core will pick it up too. <dependency> <groupId>org.slf4j</groupId> <artifactId>nlog4j</artifactId> <version>1.2.14</version> <url> http://slf4j.org/nlog4j </url> </dependency> Cheers all, Nick
        Hide
        Ceki Gulcu added a comment -

        Nick,

        I appreciate your work on setting up nlog4j at ibiblio. Thank you.

        You might want to consider using format patterns as described in [1, 2].
        Just to let you know an alternative exists,

        [1] http://www.slf4j.org/manual.html (see section "Typical usage pattern")
        [2] http://www.slf4j.org/faq.html#2.3

        Cheers,

        Show
        Ceki Gulcu added a comment - Nick, I appreciate your work on setting up nlog4j at ibiblio. Thank you. You might want to consider using format patterns as described in [1, 2] . Just to let you know an alternative exists, [1] http://www.slf4j.org/manual.html (see section "Typical usage pattern") [2] http://www.slf4j.org/faq.html#2.3 Cheers,
        Hide
        Alex Karasulu added a comment -

        Committed revision 225440.

        Show
        Alex Karasulu added a comment - Committed revision 225440.
        Hide
        Emmanuel Lecharny added a comment -

        Closing all issues created in 2005 and before which are marked resolved

        Show
        Emmanuel Lecharny added a comment - Closing all issues created in 2005 and before which are marked resolved

          People

          • Assignee:
            Alex Karasulu
            Reporter:
            Nick Faiz
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development