Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Windows][melodic] Python 3 compatibility. #1819

Merged
merged 2 commits into from
Jan 9, 2020

Conversation

seanyen
Copy link
Contributor

@seanyen seanyen commented Oct 1, 2019

On Python3 & Windows, the code path of socketserver.ForkingMixIn will be exercised and reported as an exception (shown below) because it is not available on Windows.

(py37) c:\workspace\conda_ros>roscore
Traceback (most recent call last):
  File "C:\workspace\conda_ros\install_isolated\bin\roscore", line 36, in <module>
    from rosmaster.master_api import NUM_WORKERS
  File "C:\workspace\conda_ros\install_isolated\lib\site-packages\rosmaster\__init__.py", line 35, in <module>
    from .main import rosmaster_main
  File "C:\workspace\conda_ros\install_isolated\lib\site-packages\rosmaster\main.py", line 43, in <module>
    import rosmaster.master
  File "C:\workspace\conda_ros\install_isolated\lib\site-packages\rosmaster\master.py", line 45, in <module>
    import rosgraph.xmlrpc
  File "C:\workspace\conda_ros\install_isolated\lib\site-packages\rosgraph\xmlrpc.py", line 133, in <module>
    class ForkingXMLRPCServer(socketserver.ForkingMixIn, SimpleXMLRPCServer):
AttributeError: module 'socketserver' has no attribute 'ForkingMixIn'

In this pull request, I am proposing to use conditional check for it.

@seanyen
Copy link
Contributor Author

seanyen commented Oct 1, 2019

cc @dirk-thomas

@seanyen
Copy link
Contributor Author

seanyen commented Oct 1, 2019

The test failure seems not to do with this change:

float division by zero
  File "/usr/lib/python2.7/unittest/case.py", line 329, in run
    testMethod()
  File "/tmp/ws/src/ros_comm/test/test_rospy/test/rostest/test_topic_statistics.py", line 91, in test_frequencies
    self.assertTrue(self.frequency_acceptable('/very_slow_chatter', 0.5))
  File "/tmp/ws/src/ros_comm/test/test_rospy/test/rostest/test_topic_statistics.py", line 70, in frequency_acceptable
    found_freq = 1.0 / msg.period_mean.to_sec()

@seanyen
Copy link
Contributor Author

seanyen commented Jan 8, 2020

@mikepurvis Happy new year! Hope I am pinging the correct owner. This one is ready for review and merge.

Copy link
Member

@mikepurvis mikepurvis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see no instances of ForkingXMLRPCServer actually being used in our codebase; I don't know the history of why it's here. Generating it only optionally seems perfectly fine.

tools/rosgraph/src/rosgraph/xmlrpc.py Outdated Show resolved Hide resolved
@mikepurvis
Copy link
Member

Great, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants