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 <[email protected]> wrote: > 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 <[email protected]> > wrote: >> >> > Hi, >> > >> > My name is Sreenatha. I've made an attempt to migrate Tor Weather >> > from TorCtl to Stem as stated in my gsoc proposal(Module 2). >> > >> > Can you please take a look at the top 3 commits that I've made at my >> > github account? >> > Please suggest any additional changes that you think are necessary. >> > >> > Cheers, >> > Sreenatha >> > >> > P.S : I am lucyd@OFTC on IRC. >> >> 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/ff22e74c247bd61163a974329cd2feb2a121db94#L0R26 >> >> 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 > > _______________________________________________ tor-dev mailing list [email protected] https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-dev
