commit faae224eb833d7612e2779e34d0f1010f8cb2d08 Author: Damian Johnson atagar@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/9695DFC35FFEB861329B9F1AB04C46397020CE...', timeout = 5,