From 6b90400f174be0631cd5050d5dc5964bae174cb2 Mon Sep 17 00:00:00 2001 From: Thomas Leonard Date: Sat, 7 Jan 2012 13:58:05 +0000 Subject: [PATCH] Auto-throw exceptions when resuming tasks Before, whenever a task waited for a blocker it had to call tasks.check(blocker) afterwards to check whether the blocker had an exception. Now, taking advantage of generator.throw in Python 2.6, the exception is raised directly from the yield statement. When upgrading existing code, it is always safe to transform: yield foo ... into try: yield foo except: pass ... Normally, however, you can simply eliminate the check, so: yield foo tasks.check(foo) becomes just: yield foo --- zeroinstall/0launch-gui/iface_browser.py | 3 +-- zeroinstall/0launch-gui/main.py | 9 ++++---- zeroinstall/0launch-gui/mainwindow.py | 1 - zeroinstall/0launch-gui/properties.py | 8 ++++---- zeroinstall/cmd/import.py | 2 -- zeroinstall/injector/driver.py | 7 ++++--- zeroinstall/injector/fetch.py | 35 +++++++++++++------------------- zeroinstall/injector/handler.py | 17 ++++++++++++---- zeroinstall/injector/iface_cache.py | 5 ++++- zeroinstall/injector/packagekit.py | 5 ++++- zeroinstall/injector/scheduler.py | 6 ++++-- zeroinstall/injector/selections.py | 2 -- zeroinstall/injector/trust.py | 6 ++++-- zeroinstall/support/tasks.py | 19 +++++++++++++---- 14 files changed, 72 insertions(+), 53 deletions(-) diff --git a/zeroinstall/0launch-gui/iface_browser.py b/zeroinstall/0launch-gui/iface_browser.py index 6a03897..01a1972 100644 --- a/zeroinstall/0launch-gui/iface_browser.py +++ b/zeroinstall/0launch-gui/iface_browser.py @@ -290,9 +290,8 @@ class InterfaceBrowser: @tasks.async def update_display(): - yield fetcher try: - tasks.check(fetcher) + yield fetcher # Try to insert new icon into the cache # If it fails, we'll be left with None in the cached_icon so # we don't try again. diff --git a/zeroinstall/0launch-gui/main.py b/zeroinstall/0launch-gui/main.py index a21c38e..d720018 100644 --- a/zeroinstall/0launch-gui/main.py +++ b/zeroinstall/0launch-gui/main.py @@ -126,11 +126,12 @@ def run_gui(args): if not window.systray_icon: window.show() - yield solved try: - window.refresh_button.set_sensitive(True) - window.browser.highlight_problems() - tasks.check(solved) + try: + yield solved + finally: + window.refresh_button.set_sensitive(True) + window.browser.highlight_problems() except Exception as ex: window.report_exception(ex) diff --git a/zeroinstall/0launch-gui/mainwindow.py b/zeroinstall/0launch-gui/mainwindow.py index c094356..76edc6a 100644 --- a/zeroinstall/0launch-gui/mainwindow.py +++ b/zeroinstall/0launch-gui/mainwindow.py @@ -107,7 +107,6 @@ class MainWindow: # We need to wait until everything is downloaded... blockers = [downloaded, cancelled] yield blockers - tasks.check(blockers) if cancelled.happened: return diff --git a/zeroinstall/0launch-gui/properties.py b/zeroinstall/0launch-gui/properties.py index 8de418d..23fff46 100644 --- a/zeroinstall/0launch-gui/properties.py +++ b/zeroinstall/0launch-gui/properties.py @@ -379,7 +379,6 @@ def add_remote_feed(policy, parent, interface): while True: got_response = DialogResponse(d) yield got_response - tasks.check(got_response) resp = got_response.response error(None) @@ -391,9 +390,10 @@ def add_remote_feed(policy, parent, interface): fetch = policy.fetcher.download_and_import_feed(url, iface_cache) if fetch: d.set_sensitive(False) - yield fetch - d.set_sensitive(True) - tasks.check(fetch) + try: + yield fetch + finally: + d.set_sensitive(True) iface = iface_cache.get_interface(url) diff --git a/zeroinstall/cmd/import.py b/zeroinstall/cmd/import.py index 07907da..6a51477 100644 --- a/zeroinstall/cmd/import.py +++ b/zeroinstall/cmd/import.py @@ -42,12 +42,10 @@ def handle(config, options, args): def run(): keys_downloaded = tasks.Task(pending.download_keys(config.fetcher), "download keys") yield keys_downloaded.finished - tasks.check(keys_downloaded.finished) if not config.iface_cache.update_feed_if_trusted(uri, pending.sigs, pending.new_xml): blocker = config.trust_mgr.confirm_keys(pending) if blocker: yield blocker - tasks.check(blocker) if not config.iface_cache.update_feed_if_trusted(uri, pending.sigs, pending.new_xml): raise SafeException(_("No signing keys trusted; not importing")) diff --git a/zeroinstall/injector/driver.py b/zeroinstall/injector/driver.py index 3b17a38..c291195 100644 --- a/zeroinstall/injector/driver.py +++ b/zeroinstall/injector/driver.py @@ -136,7 +136,10 @@ class Driver(object): # Wait for at least one download to finish blockers = downloads_in_progress.values() - yield blockers + try: + yield blockers + except: + pass tasks.check(blockers, self.config.handler.report_error) for f in downloads_in_progress.keys(): @@ -160,7 +163,6 @@ class Driver(object): refreshed = self.solve_with_downloads(refresh) if refreshed: yield refreshed - tasks.check(refreshed) if not self.solver.ready: raise self.solver.get_failure_reason() @@ -169,7 +171,6 @@ class Driver(object): downloaded = self.download_uncached_implementations() if downloaded: yield downloaded - tasks.check(downloaded) def need_download(self): """Decide whether we need to download anything (but don't do it!) diff --git a/zeroinstall/injector/fetch.py b/zeroinstall/injector/fetch.py index 0bf6bc5..fd4642d 100644 --- a/zeroinstall/injector/fetch.py +++ b/zeroinstall/injector/fetch.py @@ -62,9 +62,10 @@ class KeyInfoFetcher: def fetch_key_info(): try: tempfile = dl.tempfile - yield dl.downloaded - self.blocker = None - tasks.check(dl.downloaded) + try: + yield dl.downloaded + finally: + self.blocker = None tempfile.seek(0) doc = minidom.parse(tempfile) if doc.documentElement.localName != 'key-lookup': @@ -124,7 +125,6 @@ class Fetcher(object): while blockers: yield blockers - tasks.check(blockers) blockers = [b for b in blockers if not b.happened] from zeroinstall.zerostore import unpack @@ -170,7 +170,6 @@ class Fetcher(object): fetch = self.config.iface_cache.distro.fetch_candidates(master_feed) if fetch: yield fetch - tasks.check(fetch) # Force feed to be regenerated with the new information self.config.iface_cache.get_feed(feed_url, force = True) @@ -199,11 +198,8 @@ class Fetcher(object): # Download just the upstream feed, unless it takes too long... timeout = tasks.TimeoutBlocker(5, 'Mirror timeout') # 5 seconds - yield primary, timeout - tasks.check(timeout) - try: - tasks.check(primary) + yield primary, timeout if primary.happened: return # OK, primary succeeded! # OK, maybe it's just being slow... @@ -229,8 +225,10 @@ class Fetcher(object): blockers = filter(None, [primary, mirror]) if not blockers: break - yield blockers - + try: + yield blockers + except: + pass if primary: try: tasks.check(primary) @@ -283,7 +281,6 @@ class Fetcher(object): @tasks.named_async("fetch_feed " + url) def fetch_feed(): yield dl.downloaded - tasks.check(dl.downloaded) pending = PendingFeed(feed_url, stream) @@ -295,13 +292,11 @@ class Fetcher(object): keys_downloaded = tasks.Task(pending.download_keys(self, feed_hint = feed_url, key_mirror = key_mirror), _("download keys for %s") % feed_url) yield keys_downloaded.finished - tasks.check(keys_downloaded.finished) if not self.config.iface_cache.update_feed_if_trusted(pending.url, pending.sigs, pending.new_xml): blocker = self.config.trust_mgr.confirm_keys(pending) if blocker: yield blocker - tasks.check(blocker) if not self.config.iface_cache.update_feed_if_trusted(pending.url, pending.sigs, pending.new_xml): raise NoTrustedKeys(_("No signing keys trusted; not importing")) @@ -354,14 +349,12 @@ class Fetcher(object): if isinstance(retrieval_method, DownloadSource): blocker, stream = self.download_archive(retrieval_method, force = force, impl_hint = impl) yield blocker - tasks.check(blocker) stream.seek(0) self._add_to_cache(required_digest, stores, retrieval_method, stream) elif isinstance(retrieval_method, Recipe): blocker = self.cook(required_digest, retrieval_method, stores, force, impl_hint = impl) yield blocker - tasks.check(blocker) else: raise Exception(_("Unknown download type for '%s'") % retrieval_method) @@ -427,9 +420,8 @@ class Fetcher(object): @tasks.async def download_and_add_icon(): stream = dl.tempfile - yield dl.downloaded try: - tasks.check(dl.downloaded) + yield dl.downloaded if dl.unmodified: return stream.seek(0) @@ -472,7 +464,6 @@ class Fetcher(object): 'computer or affect other users. You may be asked to enter a password to confirm. The ' 'packages are:\n\n') + ('\n'.join('- ' + x for x in unsafe_impls))) yield confirm - tasks.check(confirm) blockers = [] @@ -487,8 +478,10 @@ class Fetcher(object): else: error.append((ex, tb)) while blockers: - yield blockers - tasks.check(blockers, dl_error) + try: + yield blockers + except: + tasks.check(blockers, dl_error) blockers = [b for b in blockers if not b.happened] if error: diff --git a/zeroinstall/injector/handler.py b/zeroinstall/injector/handler.py index 0896163..70b12a6 100644 --- a/zeroinstall/injector/handler.py +++ b/zeroinstall/injector/handler.py @@ -54,8 +54,11 @@ class Handler(object): @tasks.async def download_done_stats(): - yield dl.downloaded - # NB: we don't check for exceptions here; someone else should be doing that + try: + yield dl.downloaded + except: + # NB: we don't check for exceptions here; someone else should be doing that + pass try: self.n_completed_downloads += 1 self.total_bytes_downloaded += dl.get_bytes_downloaded_so_far() @@ -129,7 +132,10 @@ class Handler(object): for kf in key_info_fetchers: print(kf.status, file=sys.stderr) stdin = tasks.InputBlocker(0, 'console') blockers = [kf.blocker for kf in key_info_fetchers] + [stdin] - yield blockers + try: + yield blockers + except: + pass for b in blockers: try: tasks.check(b) @@ -262,7 +268,10 @@ class ConsoleHandler(Handler): blocker = Handler.confirm_import_feed(self, pending, valid_sigs) @tasks.async def enable(): - yield blocker + try: + yield blocker + except: + pass self.disable_progress -= 1 self.show_progress() enable() diff --git a/zeroinstall/injector/iface_cache.py b/zeroinstall/injector/iface_cache.py index 035885a..4421079 100644 --- a/zeroinstall/injector/iface_cache.py +++ b/zeroinstall/injector/iface_cache.py @@ -103,7 +103,10 @@ class PendingFeed(object): from zeroinstall.support import tasks while blockers: - yield blockers + try: + yield blockers + except: + pass old_blockers = blockers blockers = [] diff --git a/zeroinstall/injector/packagekit.py b/zeroinstall/injector/packagekit.py index ef4f5aa..0aa29f7 100644 --- a/zeroinstall/injector/packagekit.py +++ b/zeroinstall/injector/packagekit.py @@ -171,7 +171,10 @@ class PackageKit(object): do_batch(next_batch) while in_progress: - yield in_progress + try: + yield in_progress + except: + pass in_progress = [b for b in in_progress if not b.happened] class PackageKitDownload: diff --git a/zeroinstall/injector/scheduler.py b/zeroinstall/injector/scheduler.py index 3c276a6..3e53529 100644 --- a/zeroinstall/injector/scheduler.py +++ b/zeroinstall/injector/scheduler.py @@ -52,7 +52,6 @@ class DownloadScheduler: step.url = current_url blocker = self._sites[site_key].download(step) yield blocker - tasks.check(blocker) if not step.redirect: break @@ -87,7 +86,10 @@ class Site: child.start() # Wait for child to complete download. - yield thread_blocker, step.dl._aborted + try: + yield thread_blocker, step.dl._aborted + except: + pass if step.dl._aborted.happened: # Don't wait for child to finish (might be stuck doing IO) diff --git a/zeroinstall/injector/selections.py b/zeroinstall/injector/selections.py index 2c6b210..9a8be87 100644 --- a/zeroinstall/injector/selections.py +++ b/zeroinstall/injector/selections.py @@ -371,7 +371,6 @@ class Selections(object): if feed is None or sel.id not in feed.implementations: fetch_feed = config.fetcher.download_and_import_feed(feed_url, iface_cache) yield fetch_feed - tasks.check(fetch_feed) feed = iface_cache.get_feed(feed_url) assert feed, "Failed to get feed for %s" % feed_url @@ -380,7 +379,6 @@ class Selections(object): fetch_impls = config.fetcher.download_impls(needed_impls, stores) yield fetch_impls - tasks.check(fetch_impls) return download() # These (deprecated) methods are to make a Selections object look like the old Policy.implementation map... diff --git a/zeroinstall/injector/trust.py b/zeroinstall/injector/trust.py index 7fbedd0..206a25c 100644 --- a/zeroinstall/injector/trust.py +++ b/zeroinstall/injector/trust.py @@ -216,7 +216,10 @@ class TrustMgr(object): if not key_info_blockers: break info("Waiting for response from key-info server: %s", key_info_blockers) - yield [timeout] + key_info_blockers + try: + yield [timeout] + key_info_blockers + except: + pass if timeout.happened: info("Timeout waiting for key info response") break @@ -255,7 +258,6 @@ class TrustMgr(object): done = self.config.handler.confirm_import_feed(pending, kfs) if done is not None: yield done - tasks.check(done) finally: self._current_confirm = None lock.trigger() diff --git a/zeroinstall/support/tasks.py b/zeroinstall/support/tasks.py index e03ceaa..0a1ffae 100644 --- a/zeroinstall/support/tasks.py +++ b/zeroinstall/support/tasks.py @@ -248,12 +248,21 @@ class Task: info(_("Scheduling new task: %s"), self) def _resume(self): - # Remove from our blockers' queues + ex = None + # Remove from our blockers' queues and check for exceptions for blocker in self._zero_blockers: + if blocker.exception: + if ex: + warn(_("Multiple exceptions waiting; skipping %s"), ex[0]) + else: + ex = blocker.exception blocker.remove_task(self) # Resume the task try: - new_blockers = self.iterator.next() + if ex is None: + new_blockers = self.iterator.next() + else: + new_blockers = self.iterator.throw(ex[0], None, ex[1]) except StopIteration: # Task ended self.finished.trigger() @@ -359,8 +368,10 @@ def wait_for_blocker(blocker): if not blocker.happened: def quitter(): - yield blocker - wait_for_blocker.loop.quit() + try: + yield blocker + finally: + wait_for_blocker.loop.quit() Task(quitter(), "quitter") wait_for_blocker.loop = gobject.MainLoop(gobject.main_context_default()) -- 2.11.4.GIT