Details
-
Type:
Bug
-
Status: Done
-
Resolution: Done
-
Fix Version/s: None
-
Labels:
-
Story Points:2
-
Epic Link:
-
Team:Alert Production
Description
There are two bugs in the log package:
log.info("%s") results in:
TypeError: not enough arguments for format string
|
this is because Log._log is defined with this line:
self.log(level, os.path.split(frame.f_code.co_filename)[1],
|
inspect.stack()[2][3], frame.f_lineno, fmt % args)
|
and in this case args is an empty tuple. A trivial fix is to replace this with:
msg = fmt % args if args else fmt
|
self.log(level, os.path.split(frame.f_code.co_filename)[1],
|
inspect.stack()[2][3], frame.f_lineno, msg)
|
However, doing so exposes another bug. The following will segfault:
log.log(2000, "foo", "bar", 5, "%s")
|
This is because Log.log is extended in logLib.i as follows:
void log(int level, std::string const& filename,
|
std::string const& funcname, unsigned int lineno,
|
std::string const& msg) {
|
self->log(log4cxx::Level::toLevel(level),
|
log4cxx::spi::LocationInfo(filename.c_str(), funcname.c_str(), lineno),
|
msg.c_str());
|
and the version of Log.log it calls expects additional format arguments. In general the log package seems to use logMsg when no %s formatting is wanted, and log when such formatting is wanted, so I propose to fix this using:
void logMsg(int level, std::string const& filename,
|
std::string const& funcname, unsigned int lineno,
|
std::string const& msg) {
|
self->logMsg(log4cxx::Level::toLevel(level),
|
log4cxx::spi::LocationInfo(filename.c_str(), funcname.c_str(), lineno),
|
msg.c_str());
|
In addition, this requires the first fix to be changed to call logMsg instead of log
It's not a bug in log. The problem is that queryStr contains % symbols and the string passed to log.info has already had its formatting expanded to a single string object but that string object now contains % and the logger tries to expand those again with a string formatter. This is why pipe_tasks recently had lots of commits to defer the string formatting to the log package. The fix is to change the self.log.info line that is failing to use the new style:
self.log.info("queryStr=%r; dataTuple=%s", queryStr, dataTuple)
This subtle change may cause problems elsewhere until all the code understands where string format expansion is meant to occur.