[tor-commits] [stem/master] Query's run() method didn't block

[atagar at torproject.org]

commit faae224eb833d7612e2779e34d0f1010f8cb2d08
Author: Damian Johnson <atagar at torproject.org>
Date:   Sun Aug 4 11:31:05 2013 -0700

    Query's run() method didn't block
    
    Well, that was confusing as hell. Say you wrote the following script...
    
      query = stem.descriptor.remote.Query(
        '/tor/server/all.z',
        timeout = 60,
      )
    
      query.run(True)
    
      if not query.error:
        print "Results downloaded: %i" % len(list(query))
      else:
        print "Got an error: %s" % query.error
    
    You'd expect run() to block, right? You should - that's what it was documented
    as doing. Instead it returned an iterator and we don't block until the
    list(query) call. This means that we would *never* report an error. This has
    been causing stacktraces for my monitors every time it encountered an error.
---
 stem/descriptor/remote.py      |    7 +++++--
 test/unit/descriptor/remote.py |    4 ++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/stem/descriptor/remote.py b/stem/descriptor/remote.py
index 698e141..30ac578 100644
--- a/stem/descriptor/remote.py
+++ b/stem/descriptor/remote.py
@@ -275,7 +275,7 @@ class Query(object):
 
     :param bool suppress: avoids raising exceptions if **True**
 
-    :returns: iterator for the requested :class:`~stem.descriptor.__init__.Descriptor` instances
+    :returns: list for the requested :class:`~stem.descriptor.__init__.Descriptor` instances
 
     :raises:
       Using the iterator can fail with the following if **suppress** is
@@ -289,6 +289,9 @@ class Query(object):
       which case we'll pass it along.
     """
 
+    return list(self._run(suppress))
+
+  def _run(self, suppress):
     with self._downloader_thread_lock:
       self.start()
       self._downloader_thread.join()
@@ -317,7 +320,7 @@ class Query(object):
           raise self.error
 
   def __iter__(self):
-    for desc in self.run(True):
+    for desc in self._run(True):
       yield desc
 
   def _pick_url(self, use_authority = False):
diff --git a/test/unit/descriptor/remote.py b/test/unit/descriptor/remote.py
index 3ea303b..0d57dc3 100644
--- a/test/unit/descriptor/remote.py
+++ b/test/unit/descriptor/remote.py
@@ -103,7 +103,7 @@ class TestDescriptorDownloader(unittest.TestCase):
 
     # check via the run() method
 
-    self.assertRaises(ValueError, list, query.run())
+    self.assertRaises(ValueError, query.run)
 
   @patch('urllib2.urlopen')
   def test_query_with_timeout(self, urlopen_mock):
@@ -117,7 +117,7 @@ class TestDescriptorDownloader(unittest.TestCase):
       timeout = 5,
     )
 
-    self.assertRaises(socket.timeout, list, query.run())
+    self.assertRaises(socket.timeout, query.run)
     urlopen_mock.assert_called_with(
       'http://128.31.0.39:9131/tor/server/fp/9695DFC35FFEB861329B9F1AB04C46397020CE31',
       timeout = 5,