Log4php
  1. Log4php
  2. LOG4PHP-162

Warning for invalid appender threshold level never called

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.0
    • Fix Version/s: 2.2.0
    • Component/s: Code
    • Labels:
      None

      Description

      appender config:

      <appender name="foo" class="LoggerAppenderConsole" threshold="FOO" />

      Expected warning: "Invalid threshold value [FOO]..."

      LoggerLevel::toLevel() return always level-debug if default-level is null. If the threshold unknow the default-level (debug in this case) will be returned. There is no need for this warning in line 266-267 LoggerConfigurator.

        Activity

        Hide
        Ivan Habunek added a comment -

        I have fixed this and added a few more tests.

        Show
        Ivan Habunek added a comment - I have fixed this and added a few more tests.
        Hide
        Ivan Habunek added a comment -

        In fact, looking at it, I think that we should fix LoggerLevel::toLevel() so that it does not return DEBUG level when defaultLevel is set to null. I believe that it should return null in that case.

        The configurator can issue a warning and handle the situation (either use default level, or something else).

        Show
        Ivan Habunek added a comment - In fact, looking at it, I think that we should fix LoggerLevel::toLevel() so that it does not return DEBUG level when defaultLevel is set to null. I believe that it should return null in that case. The configurator can issue a warning and handle the situation (either use default level, or something else).
        Hide
        Ivan Habunek added a comment -

        Just got around to actually reading your suggestion.

        Why would you remove warnings for invalid levels? I think it would be good to notify the user that the level they entered is invalid and that the value is being ignored. Better than just doing this silently.

        Show
        Ivan Habunek added a comment - Just got around to actually reading your suggestion. Why would you remove warnings for invalid levels? I think it would be good to notify the user that the level they entered is invalid and that the value is being ignored. Better than just doing this silently.
        Hide
        Ivan Habunek added a comment -

        Aha. Sorry.
        I'm a bit busy at the moment, will have a proper look at some point in the near future.

        Show
        Ivan Habunek added a comment - Aha. Sorry. I'm a bit busy at the moment, will have a proper look at some point in the near future.
        Hide
        Florian Semm added a comment - - edited

        it's not a patch, it's a code example

        this snipped shows another place where the warning should be removed

        Show
        Florian Semm added a comment - - edited it's not a patch, it's a code example this snipped shows another place where the warning should be removed
        Hide
        Ivan Habunek added a comment -

        Thanks for the patch. Looks good to me. I'll apply it when I get a little time, which may or may not be today.

        Show
        Ivan Habunek added a comment - Thanks for the patch. Looks good to me. I'll apply it when I get a little time, which may or may not be today.
        Hide
        Florian Semm added a comment - - edited

        i think all warnings which related to invalid-levels could be removed.

        for instance the root-logger config :

        if (isset($config['level'])) {
        $level = LoggerLevel::toLevel($config['level']);
        if (isset($level))

        { $logger->setLevel($level); }

        else {
        $default = $logger->getLevel();
        $this->warn("Invalid logger level [{$config['level']}] specified for logger [$loggerName].");
        }
        }

        $level is always set.

        Show
        Florian Semm added a comment - - edited i think all warnings which related to invalid-levels could be removed. for instance the root-logger config : if (isset($config ['level'] )) { $level = LoggerLevel::toLevel($config ['level'] ); if (isset($level)) { $logger->setLevel($level); } else { $default = $logger->getLevel(); $this->warn("Invalid logger level [{$config ['level'] }] specified for logger [$loggerName] ."); } } $level is always set.

          People

          • Assignee:
            Ivan Habunek
            Reporter:
            Florian Semm
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development