Re: [tor-dev] Migration of Tor Weather to Stem
Hi Sreenatha, glad that you've started in on this! We'd much prefer for development discussions to be on tor-dev@ so looping that in. This looks like a great start. I suspect there is a little confusion though around the socket object that the Controller uses. The Controller doesn't take a socket.socket instance, but rather a stem.socket.ControlSocket... https://stem.torproject.org/api/socket.html This comes up a couple places, most notably... https://github.com/lucyd/weather/commit/ff22e74c247bd61163a974329cd2feb2a121... I doubt that function presently works. Rather, it should be... def listen(): controller = Controller.from_port(port = config.control_port) controller.authenticate(config.authenticator) controller.add_event_listener(newconsensus_listener, EventType.NEWCONSENSUS) I'm not sure what 'config.authenticator' is so that part might be wrong. Also note that this creates a control socket then forgets about it (Weather never cleans it up). This isn't a bug you've introduced but rather what Weather already did. Not the end of the world (interpreter shutdown will clean it up, though possibly with a stacktrace about lingering threads). Have you gotten a Weather instance up and running to test your changes? Cheers! -Damian
Hi Damian, Thanks for pointing out the controller socket error. I've removed my old commits and added new ones at https://github.com/lucyd/weather/commits/master. Can you please take a look at them? I haven't got a weather instance up but I followed the instructions in the README to test it and 2 of the 9 tests failed. ====================================================================== FAIL: Test a subscribe attempt to all subscription types, relying ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/lucyd/Desktop/weather/weather/../weather/weatherapp/tests.py", line 311, in test_subscribe_all self.assertEqual(node_down_sub.grace_pd, 1) AssertionError: 0 != 1 ====================================================================== FAIL: Test a node down subscription (all other subscriptions off) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/lucyd/Desktop/weather/weather/../weather/weatherapp/tests.py", line 89, in test_subscribe_node_down self.assertEqual(node_down_sub.grace_pd, 1) AssertionError: 0 != 1 ---------------------------------------------------------------------- It seems that an incorrect assert is causing the tests to fail. self.assertEqual(node_down_sub.grace_pd, 1) I looked it up and it appears that *grace_pd* is the default number of hours which the subscriber's router must be offline before a notification is sent and it's default value is zero and since it isn't set to any value in the test, it assumed it's default value and the tests failed. I modified the asserts as below and all the tests passed. self.assertEqual(node_down_sub.grace_pd, 0) Am I missing something? Or, are those tests incorrect? Also, should I have a weather instance up and running and check manually for email notifications to subscriptions or will the above tests suffice? Cheers, Sreenatha On Sun, Jun 16, 2013 at 2:06 AM, Damian Johnson <atagar@torproject.org>wrote:
Hi Sreenatha, thanks for getting this project going! I've pushed a series of commits to further clean up weather's ctlutil.py module... https://gitweb.torproject.org/user/atagar/weather.git I'm not sure about testing this service though. Christian will know best how we should proceed. On Fri, Jul 5, 2013 at 1:40 AM, Sreenatha Bhatlapenumarthi <sreenatha.dev@gmail.com> wrote:
participants (2)
-
Damian Johnson -
Sreenatha Bhatlapenumarthi