[rospy] Write log for class method with class name for rospy #1043
Conversation
```
$ export ROSCONSOLE_FORMAT='[${severity}] [${time}]: [${node}] [${function}] ${message}'
```
```
$ python spam.py
[INFO] [1472155704.567361]: [/a] [<module>] a
[WARN] [1472155704.568193]: [/a] [<module>] a
[FATAL] [1472155704.568894]: [/a] [<module>] a
[ERROR] [1472155704.569581]: [/a] [<module>] a
[INFO] [1472155704.570351]: [/a] [main] a
[WARN] [1472155704.571100]: [/a] [main] a
[FATAL] [1472155704.571878]: [/a] [main] a
[ERROR] [1472155704.572626]: [/a] [main] a
[INFO] [1472155704.573439]: [/a] [Test::__init__] a
[WARN] [1472155704.574176]: [/a] [Test::__init__] a
[FATAL] [1472155704.574937]: [/a] [Test::__init__] a
[ERROR] [1472155704.575672]: [/a] [Test::__init__] a
```
```
$ cat spam.py
import rospy
rospy.init_node('a')
rospy.loginfo('a')
rospy.logwarn('a')
rospy.logfatal('a')
rospy.logerr('a')
def main():
rospy.loginfo('a')
rospy.logwarn('a')
rospy.logfatal('a')
rospy.logerr('a')
main()
class Test(object):
def __init__(self):
rospy.loginfo('a')
rospy.logwarn('a')
rospy.logfatal('a')
rospy.logerr('a')
Test()
```
This reverts commit 6ac0137.
|
Not sure why the test failed, no tests are marked as failed, as far as I can see.. any hint ? |
|
The referenced job has one failing test: http://build.ros.org/job/Lpr__ros_comm__ubuntu_xenial_amd64/94/testReport/junit/test/test_roslogging_user_logger/test_roslogging_user_logger/ |
|
Ah thanks, I was digging through the log and didn't pay attention to the main build page... I can confirm the test behaves as expected, when launched alone, on my local Xenial machine. It seems that the problem in the test might come from the logging module from the python library. if name in self.loggerDict:
rv = self.loggerDict[name]
if isinstance(rv, PlaceHolder):
ph = rv
rv = (self.loggerClass or _loggerClass)(name)
rv.manager = self
self.loggerDict[name] = rv
self._fixupChildren(ph, rv)
self._fixupParents(rv)
else:
rv = (self.loggerClass or _loggerClass)(name)
rv.manager = self
self.loggerDict[name] = rv
self._fixupParents(rv)So I suspect that when launched with other tests, our current test will fail because the Multiple solutions :
The latter seems the cleaner to me so I ll work on that. |
| func_name = '%s.%s' % (class_name, func_name) | ||
| except KeyError: | ||
| pass | ||
| return file_name, lineno, func_name |
There was a problem hiding this comment.
I think it would be good to call the implementation of the base class here and then only "enhance" the function name with additional information.
|
@ros-pull-request-builder retest this please |
| def findCaller(self): | ||
| """ | ||
| Find the stack frame of the caller so that we can note the source | ||
| file name, line number, and function name with class name if possible. |
There was a problem hiding this comment.
While this is a good description of the function itself would it be better to document what this specific variation does (extending the function name with the class name if applicable)?
|
I believe it is already written there ?
…--
AlexV
On Fri, May 26, 2017 at 5:31 AM, Dirk Thomas ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tools/rosgraph/src/rosgraph/roslogging.py
<#1043 (comment)>:
>
import rospkg
from rospkg.environment import ROS_LOG_DIR
class LoggingException(Exception): pass
+class RospyLogger(logging.getLoggerClass()):
+ def findCaller(self):
+ """
+ Find the stack frame of the caller so that we can note the source
+ file name, line number, and function name with class name if possible.
While this is a good description of the function itself would it be better
to document what this specific variation does (extending the function name
with the class name if applicable)?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1043 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AANgSKOIf69lK-dYYZVBsDqm9m1v5n3-ks5r9eU4gaJpZM4NNxq_>
.
|
|
@asmodehn Thank you for addressing this feature. 👍 |
|
I am sorry for the late response. This looks good. Merging... |
* Write log for class method with class name for rospy
```
$ export ROSCONSOLE_FORMAT='[${severity}] [${time}]: [${node}] [${function}] ${message}'
```
```
$ python spam.py
[INFO] [1472155704.567361]: [/a] [<module>] a
[WARN] [1472155704.568193]: [/a] [<module>] a
[FATAL] [1472155704.568894]: [/a] [<module>] a
[ERROR] [1472155704.569581]: [/a] [<module>] a
[INFO] [1472155704.570351]: [/a] [main] a
[WARN] [1472155704.571100]: [/a] [main] a
[FATAL] [1472155704.571878]: [/a] [main] a
[ERROR] [1472155704.572626]: [/a] [main] a
[INFO] [1472155704.573439]: [/a] [Test::__init__] a
[WARN] [1472155704.574176]: [/a] [Test::__init__] a
[FATAL] [1472155704.574937]: [/a] [Test::__init__] a
[ERROR] [1472155704.575672]: [/a] [Test::__init__] a
```
```
$ cat spam.py
import rospy
rospy.init_node('a')
rospy.loginfo('a')
rospy.logwarn('a')
rospy.logfatal('a')
rospy.logerr('a')
def main():
rospy.loginfo('a')
rospy.logwarn('a')
rospy.logfatal('a')
rospy.logerr('a')
main()
class Test(object):
def __init__(self):
rospy.loginfo('a')
rospy.logwarn('a')
rospy.logfatal('a')
rospy.logerr('a')
Test()
```
* Fix func name {class_name}.{method_name}
* Get line number for rospy logger
* Add test for roslogging ROSCONSOLE_FORMAT
* Add license header
* Don't overwrite logger defined by user
* Inherit user defined custom logger class in RospyLogger
* Fix test name of test_roslogging_user_logger.py
* Make logging test as isolated one
* Revert "Make logging test as isolated one"
This reverts commit 6ac0137.
* Fix test for rosgraph with user custom logger
* using specifically named logger for user logger test.
* [rosgraph] changed logic for roslogging findCaller()
|
@dirk-thomas As far as I understand the code :
So I believe we don't want to extend rosgraph tests to tests throttled and named loggers... We probably want to extend rospy logging tests to verify the logging format in the throttled and named cases instead ? |
|
How about something like that : #1144 ? |
#877 Rebased on lunar-devel