Tasks that return status streams should be replaced with calls to IProgress<StreamStatus>

[swernli]

Methods that return a Task<Stream> like PullImageAsync could instead return Task<IProgress<Stream>> to abstract away some of the details of how docker reports it's progress. This should simplify patterns for callers who want to report that progress.

@jterry75 Can expand on what we are looking for here.

[jterry75]

Instead of returning Task<Stream> we should return Task. Then add a parameter to the function that takes IProgress<PullStatus> progress. Each time that we receive the new "stream value" we deserialize it into the PullStatus and then Report() any waiter. The client looks like this:

var p = new Progress<Docker.DotNet.PullStatus>(status =>
{
// Do something with the status? Update a UI/PS/Service on the Sync Context of your choice.
});

await PullImageAsync(/*params*/, p);

PullStatus is just a moniker. If Docker always does the same status struct then we can just use that instead for all methods that return a Task<Stream> where <Stream> contains only data that is just status. IE: docker logs is not a candidate here because the logs are an actual value.

[jterry75]

I updated the tile which was very misleading. My response above is what we actually want. Not to change the return type to return progress but rather pass in an IProgress of the appropriate type so we can handle the status reporting to waiters.

[galvesribeiro]

@jterry75 as mentioned on #143, Lets implement this issue.

I would like to align here the proper method signatures:

1. For methods that are today returning Task<Stream> we should do as you mentioned here, correct?
2. For methods that are long running monitoring, like the ones on MISC endpoint, which endlessly monitor events on the engine/swarm, I would suggest return an IObservable<T> where T is a structure with the general status update. That way, people don't need to await on that and instead can just subscribe. They can even use Rx to consume. As you may know, IObservable<T> is available on .Net so we don't need add any dependencies... What do you think?

When I got answer for those two I'll submit the PR.

Thanks

[jterry75]

1. Agreed, I think anything that returns Task other than the image downloads can be done this way.
2. Correct that is the right pattern for Long Running Tasks, I would love to see that change!

[galvesribeiro]

Perfect! Will do it tonight when get home. Thanks @jterry75

[galvesribeiro]

@jterry75 it is done on #147.

I decided to not use IObservable<T> because it will be too big breaking change on the public APIs.

I think IProgress<T> is enough for that case. Specially that most of the time, the events are limited to an small number.

Please review when you can so I can move to the others. Thanks! Appreciate the support! :)

[jterry75]

This has been merged. Self assigning to cleanup Push/PullImageAsync in the v1.13 branch given the inclusion of JSONMessage etc.

[galvesribeiro]

Thanks @jterry75 let me what else todo :)