Add ablity to redirect process output and error

[gravypower]

I wanted to display the output of a docker-compose call in a CLI I am building.

[mariotoffia]

Thanks @gravypower for this nice PR! Could you please add the ability to at least all docker-compose commands (in same file)? (it would also be nice with the other docker commands of course :))

Cheers,
Mario

[gravypower]

Thanks for the feedback @mariotoffia. I will look into your suggestion and update this PR.

[gravypower]

I started to do this and it is turning into spaghetti code :(. I might abandon that approach I am taking and reconsider it. Will report back when I have something more.

[gravypower]

Hey @mariotoffia,

I have introduced an Ambient Context to achieve this, but it makes me feel a bit dirty.

So a consumer of FluentDocker who wants to redirect output from the docker/docker-compose call would do something like this

var workingDockerCompose = Path.Combine(input.WorkingPath, "docker-compose.yml");
var builder = new Builder()
    .UseContainer()
    .UseCompose()
    .FromFile(workingDockerCompose)
    .RemoveOrphans();

var compositeService = builder.Build();
var p = new ProcessManager();

p.ErrorTextReceived += (sender, s) =>    
{
    if(sender.ProcessIdentifier != nameof(Compose.ComposeUp))
        return;
    
    if (!string.IsNullOrEmpty(s))
        Console.Error.WriteAsync(s);
};

p.StandartTextReceived += (sender, s) => 
{
    if(sender.ProcessIdentifier != nameof(Compose.ComposeUp))
        return;
    
    if (!string.IsNullOrEmpty(s))
        Console.Write(s);
};

using (ProcessManagerContext.UseProcessManager(p))
{
    compositeService.Start();
}

What do you think? I feel its the best way without sweeping changes.

Extra information:

https://blog.ploeh.dk/2019/01/21/some-thoughts-on-anti-patterns/
https://aabs.wordpress.com/2007/12/31/the-ambient-context-design-pattern-in-net/
https://startbigthinksmall.wordpress.com/2008/04/24/nice-free-and-reusable-net-ambient-context-pattern-implementation/
https://github.com/minimod/minimods/blob/master/MiniMods/ThreadVariable/ThreadVariable.cs
https://softwareengineering.stackexchange.com/questions/363696/ambient-context-vs-constructor-injection

[gravypower]

I was stuffing around trying to remove a commit might have botched the tree 😟. Looking into why I have broken the build now.

[gravypower]

OK I am not sure what is going on with the build, I have run dotnet gitversion /l console /output buildserver locally with no issue.

[mariotoffia]

@gravypower Thanks for your work! :) - I'll have a look at your ProcessManager next week. It is quite invasive, including thread variables of which makes this project a bit more complex so I would need some time. Thanks for the nice work though!

Hmm... I will also investigate why you get a merge conflict.

Cheers

[gravypower]

Oh. I will check again I thought that I had merged in master, maybe I forgot to push that. AH yes the ProcessManager, had plans to be able to redirect input and output, I should simplify this for now.

[mariotoffia]

Shouldn't it be any asserts in order to ensure that it contains the correct content?

[gravypower]

This was copied from the tests where I pulled the AmbientContext from. I can remove if you think that is required.

[mariotoffia]

please do

[mariotoffia]

Could'n we use a instance based approach for a ProcessManager instead of a static one and thus avoid thread variables (? - haven't checked what you use them for).

like use(pm = ...) {
fd.UsePm(pm).
...
}

when the pm context exits, all commands for compse and docker services, etc. will fail (with e.g. missing pm (name xyz)). Or at-least a non static approach?

It will later on fit into the event model (v4.0.0) so the pm will send events and docker events is also emitted as events into a event dispatcher that one may register to one or more services.

[gravypower]

I have simplified what is going on here This is needed however its how you access the AmbientContext. If you look under the hood of that context its getting a thread variable. Like I said I feel a bit dirty.

[mariotoffia]

I think a better approach is to skip the AmbientContext and just pass the ProcessManager into the commands instead - just use a static default if not provided?

[jalbrzym]

@mariotoffia Hello Mario! Are there any plans to finish and merge this feature into the production branch? It would provide a great experience during CI pipelines

[mariotoffia]

Hi @jalbrzym - It needs heavy refactoring (as described above) since this is too static. I have no immediate plan to do this but I can up-prioritize this if this is a very much needed feature?

Cheers,
Mario :)