Rework ArchiveAsync interface to make it return any archiving errors

[ncw]

This PR contains two commits

• Rework ArchiveAsync interface to make it return any archiving errors

After experience with the current ArchiveAsync interface while
creating an rclone backend it became clear that it wasn't quite right.

With the old interface, callers did not know when the File had been
archived and thus did not know when to release resources associated
with that file.

This patch changes the interface so that it returns the error
archiving each File. This means the caller can know when the File has
been archived and when to release resources. It returns the error for
each file archived which is useful too.

Fixes #368

• Add ArchiveAsync support to CompressedArchive

[ncw]

Having had a chance to sleep on it, I think the the interface I've proposed has a problem.

Allowing the range loop to exit on errors will cause a potential deadlock as any senders into jobs don't know whether there is a process listening or not. This isn't easily fixable in a race free way. So I think the loops should not exit on error.

So I think something like this is more correct.

func (z Zip) ArchiveAsync(ctx context.Context, output io.Writer, jobs <-chan ArchiveAsyncJob) error {
	zw := zip.NewWriter(output)
	defer zw.Close()

	var i int
	for job := range jobs {
		job.Result <- z.archiveOneFile(ctx, zw, i, job.File)
		i++
	}

	return nil
}

I'll update the PR accordingly in a moment!

[ncw]

I've added a new commit on top with that idea which I can squash into the first commit if you want.

[mholt]

Alrighty -- sorry for the wait, I was traveling with family.

I think this looks pretty good! The API is much better; and I guess context cancellation and ContinueOnError could be honored by the caller of ArchiveAsync.

My only comment is a nit and is optional. This has my approval. Thanks again!

[ncw]

I have sent a new update with the fix from above and I've also squashed the PR back into two commits with the error handling changes.

Alrighty -- sorry for the wait, I was traveling with family.

No problem. You are doing much better than me at reviewing PRs :-)

I think this looks pretty good! The API is much better; and I guess context cancellation and ContinueOnError could be honored by the caller of ArchiveAsync.

Yes its completely under the control of the caller. The current loops don't wait for context cancellation at the moment and I think if you need that it is better for the caller to do it otherwise you'll risk deadlocks when the input channel fills up because there is no-one listening.

My only comment is a nit and is optional. This has my approval. Thanks again!

👍