Merge lp:~george-edison55/charm-helpers/fix-fetch-handler into lp:charm-helpers

[Nathan Osman]

charmhelpers.fetch.install_remote() contains a logic error. The first line builds a list of handlers that match the provided source URI. In the case of an HTTP or HTTPS URL, both the ArchiveUrlFetchHandler and GitUrlFetchHandler will match and be added to the list.

The body of the loop is then executed twice - once for each handler. The first time, the ArchiveUrlFetchHandler will be run and installed_to will be set to the return value of handler.install(). However, this value will be overwritten by GitUrlFetchHandler the second time through the loop.

I have refactored the function to remove this problem. Now the function immediately returns the value from handler.install() and continues to the next handler only if an exception was raised.

I have checked my modifications using "make test" and everything seems to be in order. This addresses bug #1504346.

[Stuart Bishop (stub)]

Looks good.

review: Approve

=== modified file 'charmhelpers/fetch/__init__.py'
--- charmhelpers/fetch/__init__.py	2015-08-10 13:34:31 +0000
+++ charmhelpers/fetch/__init__.py	2015-10-09 00:17:23 +0000
@@ -382,16 +382,13 @@
     # We ONLY check for True here because can_handle may return a string
     # explaining why it can't handle a given source.
     handlers = [h for h in plugins() if h.can_handle(source) is True]
-    installed_to = None
     for handler in handlers:
         try:
-            installed_to = handler.install(source, *args, **kwargs)
+            return handler.install(source, *args, **kwargs)
         except UnhandledSource as e:
             log('Install source attempt unsuccessful: {}'.format(e),
                 level='WARNING')
-    if not installed_to:
-        raise UnhandledSource("No handler found for source {}".format(source))
-    return installed_to
+    raise UnhandledSource("No handler found for source {}".format(source))
 
 
 def install_from_config(config_var_name):