Bug 16313 - Koha::Logger doesn't forward the Log::Log4perl::Logger return value
Summary: Koha::Logger doesn't forward the Log::Log4perl::Logger return value
Status: BLOCKED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Olli-Antti Kivilahti
QA Contact: Testopia
URL:
Keywords:
Depends on: 16302 16304 16312 14167
Blocks: 16329
  Show dependency treegraph
 
Reported: 2016-04-20 17:26 UTC by Olli-Antti Kivilahti
Modified: 2017-10-15 09:39 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16313 - Koha::Logger doesn't forward the Log::Log4perl::Logger return value (2.80 KB, patch)
2016-04-20 17:29 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Olli-Antti Kivilahti 2016-04-20 17:26:39 UTC
Log::Log4perl::Logger->$method
 returns undef if the log level is not logged or something bad happened and true or something similar on success. This allows chaining different loggers to trigger if Log4perl failed to log.

Koha::Logger is hardwired to return 1 if Log4perl is configured properly regardless of whether the Log4perl actually succeeded in printing the log message.

This is a Log4perl best practice:

$Koha::Logger->debug() or syslog("LOG_DEBUG", ...);
which should revert to syslog() to log if Log4perl cannot for some reason do the logging. (eg. is disabled)

This patch makes the Koha::Logger work with Log4perl better in tandem and return the proper return values.
Comment 1 Olli-Antti Kivilahti 2016-04-20 17:29:11 UTC
Created attachment 50473 [details] [review]
Bug 16313 - Koha::Logger doesn't forward the Log::Log4perl::Logger return value

    Log::Log4perl::Logger->$method
returns undef if the log level is not logged or something bad happened and
true or something similar on success. This allows chaining different loggers
to trigger if Log4perl failed to log.

Koha::Logger is hardwired to return 1 if Log4perl is configured properly
regardless of whether the Log4perl actually succeeded in printing the log
message.

The following is a Log4perl best practice:

$Koha::Logger->debug() or syslog("LOG_DEBUG", ...);
which should revert to syslog() to log if Log4perl cannot for some reason do the
logging. (eg. is disabled)

This patch makes the Koha::Logger work with Log4perl better in tandem and
return the proper return values.

See unit tests.
Comment 2 Olli-Antti Kivilahti 2016-04-20 17:33:04 UTC
If you made it this far, the correct order of applying the dependant patches is as follows:

Bug 16313 - Koha::Logger doesn't forward the Log::Log4perl::Logger return v~
Bug 16312 - Koha::Logger overload configuration for command line scripts ~
Bug 16306 - C4::Context->setCommandlineEnvironment() convenience function
Bug 16304 - Koha::Logger, lazy load loggers so environment has time to get set
Bug 16303 - Koha::Logger appenders for interface 'commandline'
Bug 16302 - Koha::Logger performance tests
Bug 14167 - Add Koha::Logger based on Log4perl.
Comment 3 Katrin Fischer 2017-10-15 09:39:08 UTC
Depends on a patch in Failed QA - Marking as BLOCKED until dependency can be resolved.