Return the file from Download.Wait().

sdg0919 · Apr 26, 2016 · aa96c51 · aa96c51
1 parent 09796b8
commit aa96c51
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 14 deletions.
diff --git a/downloader/download.go b/downloader/download.go
@@ -77,18 +77,23 @@ func (dl *Download) Done() <-chan Status {
 }
 
 // Wait blocks until the download completes or the abort channel receives.
-func (dl *Download) Wait(abort <-chan struct{}) (Status, error) {
+func (dl *Download) Wait(abort <-chan struct{}) (*os.File, error) {
 	defer dl.Stop()
 
 	select {
 	case <-abort:
 		logger.Infof("download aborted")
-		return Status{}, errors.New("aborted")
+		return nil, errors.New("aborted")
 	case status := <-dl.Done():
 		if status.Err != nil {
-			return Status{}, errors.Trace(status.Err)
+			if status.File != nil {
+				if err := status.File.Close(); err != nil {
+					logger.Errorf("failed to close file: %v", err)
+				}
+			}
+			return nil, errors.Trace(status.Err)
 		}
-		return status, nil
+		return status.File, nil
 	}
 }
 

diff --git a/downloader/downloader.go b/downloader/downloader.go
@@ -7,6 +7,7 @@ import (
 	"io"
 	"net/url"
 
+	"github.com/juju/errors"
 	"github.com/juju/loggo"
 	"github.com/juju/utils"
 )
@@ -42,8 +43,15 @@ func (dlr Downloader) Start(req Request) *Download {
 }
 
 // Download starts a new download, waits for it to complete, and
-// returns the result.
-func (dlr Downloader) Download(req Request, abort <-chan struct{}) (Status, error) {
+// returns the local name of the file.
+func (dlr Downloader) Download(req Request, abort <-chan struct{}) (filename string, err error) {
 	dl := dlr.Start(req)
-	return dl.Wait(abort)
+	file, err := dl.Wait(abort)
+	if file != nil {
+		defer file.Close()
+	}
+	if err != nil {
+		return "", errors.Trace(err)
+	}
+	return file.Name(), nil
 }
diff --git a/worker/uniter/charm/bundles.go b/worker/uniter/charm/bundles.go
@@ -19,7 +19,7 @@ import (
 type Download interface {
 	// Wait blocks until the download completes or the abort channel
 	// receives.
-	Wait(abort <-chan struct{}) (downloader.Status, error)
+	Wait(abort <-chan struct{}) (*os.File, error)
 }
 
 // BundlesDir is responsible for storing and retrieving charm bundles
@@ -95,7 +95,6 @@ func download(info BundleInfo, archiveURLs []*url.URL, targetDir string, startDo
 		return "", errors.Trace(err)
 	}
 
-	var status downloader.Status
 	for _, archiveURL := range archiveURLs {
 		logger.Infof("downloading %s from %s", info.URL(), archiveURL)
 		dl, err2 := startDownload(downloader.Request{
@@ -106,19 +105,23 @@ func download(info BundleInfo, archiveURLs []*url.URL, targetDir string, startDo
 		if err2 != nil {
 			return "", errors.Trace(err2)
 		}
-		status, err = dl.Wait(abort)
-		if status.File != nil {
-			defer status.File.Close()
+		file, err2 := dl.Wait(abort)
+		err = err2
+		if errors.IsNotValid(err2) {
+			logger.Errorf("download from %s invalid: %v", archiveURL, err2)
+			break
 		}
-		if err == nil || errors.IsNotValid(err) {
+		if err == nil {
+			defer file.Close()
+			filename = file.Name()
 			break
 		}
 		logger.Errorf("download request to %s failed: %v", archiveURL, err)
 	}
 	if err != nil {
 		return "", errors.Trace(err)
 	}
-	return status.File.Name(), nil
+	return filename, nil
 }
 
 // bundlePath returns the path to the location where the verified charm