strands-project / mongodb_store

MongoDB tools for storing and analysing runs of ROS systems.
BSD 3-Clause "New" or "Revised" License
49 stars 72 forks source link

Parameter related test fail #143

Closed hawesie closed 8 years ago

hawesie commented 9 years ago

https://github.com/strands-project/strands_executive/pull/212#issuecomment-134602697

hawesie commented 9 years ago

@marc-hanheide I don't think there is a mongodb problem here. If I launch

roslaunch mongodb_store mongodb_store.launch test_mode:=true

then

rosparam get mongodb_host

It's fine.

cburbridge commented 9 years ago

Looks like the parameters are not set in test mode. https://github.com/strands-project/mongodb_store/blob/50cb1fb3d74416f415d55e6a95032594b48adfb9/mongodb_store/launch/mongodb_store.launch#L34 and L35 maybe need to be duplicated at L27?

@hawesie is that after restarting roscore?

hawesie commented 9 years ago

The parameters are not set in the launch file. They are set in the (master) server: https://github.com/strands-project/mongodb_store/blob/hydro-devel/mongodb_store/scripts/mongodb_server.py#L68

marc-hanheide commented 9 years ago

I'm lost. The quoted line does not set the parameter. It only gives a local default.

hawesie commented 9 years ago

https://github.com/strands-project/mongodb_store/blob/hydro-devel/mongodb_store/scripts/mongodb_server.py#L70 and https://github.com/strands-project/mongodb_store/blob/hydro-devel/mongodb_store/scripts/mongodb_server.py#L72 do the setting.

marc-hanheide commented 9 years ago

Any idea why it fails then?

hawesie commented 9 years ago

It seems to be back to normal now! It looked to me like Jenkins was suffering from some network issue when the original tests were running.

https://lcas.lincoln.ac.uk/jenkins/job/devel-indigo-strands_executive/ARCH_PARAM=amd64,UBUNTU_PARAM=trusty,label=devel/95/changes

marc-hanheide commented 9 years ago

Odd... But let's close this then.

marc-hanheide commented 9 years ago

So, this is a race condition. The error happens in config_manager.py:

Traceback (most recent call last):
  File "/opt/ros/indigo/lib/mongodb_store/message_store_node.py", line 236, in <module>
    store = MessageStore()
  File "/opt/ros/indigo/lib/mongodb_store/message_store_node.py", line 26, in __init__
    db_host = rospy.get_param('mongodb_host')
  File "/opt/ros/indigo/lib/python2.7/dist-packages/rospy/client.py", line 460, in get_param
    return _param_server[param_name] #MasterProxy does all the magic for us
  File "/opt/ros/indigo/lib/python2.7/dist-packages/rospy/msproxy.py", line 123, in __getitem__
    raise KeyError(key)
KeyError: 'mongodb_host'
Traceback (most recent call last):
  File "/opt/ros/indigo/lib/mongodb_store/config_manager.py", line 271, in <module>
    server = ConfigManager()
  File "/opt/ros/indigo/lib/mongodb_store/config_manager.py", line 74, in __init__
    db_port = rospy.get_param('mongodb_port')
  File "/opt/ros/indigo/lib/python2.7/dist-packages/rospy/client.py", line 460, in get_param
    return _param_server[param_name] #MasterProxy does all the magic for us
  File "/opt/ros/indigo/lib/python2.7/dist-packages/rospy/msproxy.py", line 123, in __getitem__
    raise KeyError(key)
KeyError: 'mongodb_port'
Traceback (most recent call last):
  File "/opt/ros/indigo/lib/python2.7/dist-packages/rospy/core.py", line 399, in signal_shutdown
    h()
  File "/opt/ros/indigo/lib/mongodb_store/config_manager.py", line 210, in _on_node_shutdown
    self._mongo_client.disconnect()
AttributeError: 'ConfigManager' object has no attribute '_mongo_client'

Setting a parameter in a script is not recommended, as then it's unclear when it will be set. This just caused another strands_executive unit test to fail. We should be setting the parameter in the launch file, not the script, IMO.

hawesie commented 8 years ago

I made an update in #150 which has hopefully fixed this. I think setting the param in the script is fine, provided there is something to wait for before checking it. If components use the wait_for_mongo util method before reading the param, all should be fine.