Fixed search strategy for python_logging config#1292
Fixed search strategy for python_logging config#1292dirk-thomas merged 2 commits intoros:lunar-develfrom
Conversation
94fd2ab to
14fb677
Compare
mikepurvis
left a comment
There was a problem hiding this comment.
Seems reasonable, but can you describe more about how you verified the new and existing behaviour?
| '/etc/ros/%s'%(fname), | ||
| os.path.join(rosgraph_d, 'conf', fname)]: | ||
| for config_dir in [os.path.join(rospkg.get_ros_home(), 'config'), | ||
| '/etc/ros/', |
There was a problem hiding this comment.
Please use get_etc_ros_dir, since that accounts for the ROS_ETC_DIR envvar:
http://docs.ros.org/independent/api/rospkg/html/rospkg_environment.html#get_etc_ros_dir
| for config_dir in [os.path.join(rospkg.get_ros_home(), 'config'), | ||
| '/etc/ros/', | ||
| os.path.join(rosgraph_d, 'conf')]: | ||
| for extension in ['.conf', '.yaml', '.yml']: |
There was a problem hiding this comment.
Consider os.extsep, but it isn't used a lot elsewhere in ROS tooling, and . works on all of the supported ROS platforms.
14fb677 to
bb77dc9
Compare
|
Updated as suggested. I played a bit and placed files here and there. And from the code it was somehow obvious that when a file is found, the search ends. |
|
👍 |
|
I don't think we need to introduce the option for Thank you for the patch. This order makes much more sense. |
The current strategy tries to first find a file named python_logging.conf and, in case none was found, continues searching the yaml version.
A custom configuration in /etc/ros/python_logging.yaml would be ignored as the file python_logging.conf is always present in the share/conf folder of rosgraph.