Add async flag to siac renter download

[MSevey]

resolves #2209

[ChrisSchinnerl]

Does that still print the progress to stdout? If it is so fast that it is not noticeable we can probably leave it like that, but otherwise we can probably just avoid spawning the goroutine if renterDownloadAsync equals true.

[DavidVorick]

Wouldn't it be annoying for the progress to be printed to stdout if you were trying to do async download?

[DavidVorick]

Yeah I think sufficient to just skip the goroutine that prints out the progress

[tbenz9]

I agree it shouldn't print progress, but I think it might be nice to print a message when the file is done downloading.

[MSevey]

Ok so should it just be broken into an if statement then like this:

var err error
if renterDownloadAsync {
    err = httpClient.RenterDownloadFullGet(path, destination, renterDownloadAsync)
} else {
    done := make(chan struct{})
    go downloadprogress(done, path)
    err = httpClient.RenterDownloadFullGet(path, destination, renterDownloadAsync)
    close(done)
}

[ChrisSchinnerl]

@MSevey you can do:

done := make(chan struct{})
if !renterDownloadAsync { 
  go downloadprogress(done, path) // print progress if we don't download async
}
err = httpClient.RenterDownloadFullGet(path, destination, renterDownloadAsync)
close(done)

It doesn't really hurt to not use a channel.

[MSevey]

Ok great

[lukechampine]

I don't see much advantage to this over siac renter download >/dev/null &, but I guess that isn't as easy to do on Windows.

It might make sense to invert the goroutines here:

go func() {
    err = httpClient.RenterDownloadFullGet(path, destination, renterDownloadAsync)
    close(done)
}()
downloadprogress(done, path)

since arguably it's the download that's happening asynchronously, not the printing. In general, when structuring goroutines, I think it's best for the longest-lived goroutine to be in the main body. downloadprogress by definition outlives RenterDownloadFullGet, so putting it on the main code path ensures that all other goroutines have exited before the function returns. Whereas the way it's currently written, the downloadprogress goroutine could outlive renterfilesdownloadcmd. (Not a huge deal in this case, since the program is going to exit immediately afterward, but you get the idea.)

[DavidVorick]

The way that it's coded currently, we add an extra second of sleep for no reason to async downloads. I don't think we should call the downloadProgress function at all if the download is async.