diff --git a/beets/autotag/mb.py b/beets/autotag/mb.py index b87716051..927b0fa1a 100644 --- a/beets/autotag/mb.py +++ b/beets/autotag/mb.py @@ -30,22 +30,42 @@ import time import datetime import musicbrainz2.webservice as mbws +class ServerBusyError(Exception): pass + # MusicBrainz requires that a client does not query the server more # than once a second. This function enforces that limit using a # module-global variable to keep track of the last time a query was # sent. +MAX_QUERY_RETRY = 8 QUERY_WAIT_TIME = 1.0 last_query_time = 0.0 -def _query_wait(): +def _query_wrap(fun, *args, **kwargs): """Wait until at least `QUERY_WAIT_TIME` seconds have passed since - the last invocation of this function. This should be called (by - this module) before any query is sent. + the last invocation of this function. Then call + fun(*args, **kwargs). If it fails due to a "server busy" message, + then try again. Tries up to `MAX_QUERY_RETRY` times before + giving up. """ global last_query_time - since_last_query = time.time() - last_query_time - if since_last_query < QUERY_WAIT_TIME: - time.sleep(QUERY_WAIT_TIME - since_last_query) - last_query_time = time.time() + for i in range(MAX_QUERY_RETRY): + since_last_query = time.time() - last_query_time + if since_last_query < QUERY_WAIT_TIME: + time.sleep(QUERY_WAIT_TIME - since_last_query) + last_query_time = time.time() + try: + # Try the function. + res = fun(*args, **kwargs) + except mbws.WebServiceError, e: + # Server busy. Retry. + if 'Error 503' not in str(e.reason): + # This is not the error we're looking for. + raise + else: + # Success. Return the result. + return res + # Gave up. + raise ServerBusyError() + # FIXME exponential backoff? def _lucene_escape(text): """Escapes a string so it may be used verbatim in a Lucene query @@ -75,8 +95,7 @@ def find_releases(criteria, limit=25): # Build the filter and send the query. filt = mbws.ReleaseFilter(limit=limit, query=query) - _query_wait() - return mbws.Query().getReleases(filter=filt) + return _query_wrap(mbws.Query().getReleases, filter=filt) def release_dict(release, tracks=None): """Takes a MusicBrainz `Release` object and returns a dictionary @@ -124,8 +143,7 @@ def release_tracks(release_id): release. If the release is not found, returns an empty list. """ inc = mbws.ReleaseIncludes(tracks=True) - _query_wait() - release = mbws.Query().getReleaseById(release_id, inc) + release = _query_wrap(mbws.Query().getReleaseById, release_id, inc) if release: return release.tracks else: diff --git a/test/test_autotag.py b/test/test_autotag.py index 3b4893a75..1e6a13aa8 100755 --- a/test/test_autotag.py +++ b/test/test_autotag.py @@ -36,6 +36,7 @@ class AutotagTest(unittest.TestCase): self.assertEqual(l_artist, 'The Beatles') self.assertEqual(l_album, 'The White Album') +def nullfun(): pass class MBQueryWaitTest(unittest.TestCase): def setup(self): # simulate startup @@ -43,22 +44,22 @@ class MBQueryWaitTest(unittest.TestCase): def test_do_not_wait_initially(self): time1 = time.time() - mb._query_wait() + mb._query_wrap(nullfun) time2 = time.time() self.assertTrue(time2 - time1 < 1.0) def test_second_rapid_query_waits(self): - mb._query_wait() + mb._query_wrap(nullfun) time1 = time.time() - mb._query_wait() + mb._query_wrap(nullfun) time2 = time.time() self.assertTrue(time2 - time1 > 1.0) def test_second_distant_query_does_not_wait(self): - mb._query_wait() + mb._query_wrap(nullfun) time.sleep(1.0) time1 = time.time() - mb._query_wait() + mb._query_wrap(nullfun) time2 = time.time() self.assertTrue(time2 - time1 < 1.0)