Tyrrrz / CliFx

Class-first framework for building command-line interfaces
MIT License
1.5k stars 61 forks source link

How to know what command will be executed #86

Closed nickdurcholz closed 3 years ago

nickdurcholz commented 4 years ago

I have a use case where it would be advantageous to be able to know what command is about to execute prior to calling CliApplication.RunAsync(). My application has various sub commands, some of which are intended to be called by scripts and print json to std out, and others which are intended to be called by a person from the command line.

I would like to configure my logging library to print log output to the console for some commands and not others, and it seems like I should be able to get some sort of information about what command has been selected from CliFx before running it. This feature doesn't seem to exist in 1.4.0.

Is there any ability to get a reference to the command that CliFx runs prior to executing it? If not, would you be willing to consider a pull request that added that ability?

Tyrrrz commented 4 years ago

Hi. You can usually detect if the application is running in a terminal or programmatically by checking if the output has been redirected. You could do...

if (console.IsOutputRedirected)
{
    // Running from a script
    // Enable or disable logging
}

It may be good enough for your use case?

Currently, there is no way to get the reference to the executing command.

nickdurcholz commented 4 years ago

That doesn't exactly give me what I need. I'm less interested in whether the output is being redirected and more interested in configuring Serilog differently depending on which command I'm executing. This is what I currently have in my main method:

        var services = new ServiceCollection();
        ...
        var loggerConfig = new LoggerConfiguration()
            .ReadFrom.Configuration(config)
            .WriteTo.Async(sink => sink
                .File(Path.Combine(logDir, "InstanceManager.log"),
                    rollingInterval: RollingInterval.Day,
                    outputTemplate: "{Timestamp:o} {SourceContext} [{Level}] {Message}{NewLine}{Exception}",
                    retainedFileCountLimit: 365)
            )
            .Enrich.FromLogContext();
        if (args[0] != "request" && args[0] != "service" && args[0] != "wait")
            loggerConfig.WriteTo.Console();
        Log.Logger = loggerConfig.CreateLogger();

        var serviceProvider = services.BuildServiceProvider();

        return new CliApplicationBuilder()
            .AddCommandsFromThisAssembly()
            .UseTypeActivator(new TypeActivator(serviceProvider))
            .UseDescription("AWS Instance Manager controls the state of the EC2 instance " +
                            "in response to SQS Messages and communicates the response " +
                            "back to the deployment scripts")
            .AllowDebugMode()
            .Build()
            .RunAsync(args)
            .AsTask();

I'd like to be able to get rid of those args[0] references, and I think my options are to either defer configuration of the logging system until we actually run a command or be able to know what command has been selected prior to calling RunAsync().

I could work around it by initializing logging in the commands themselves, but the spirit of this library seemed to be enabling you to do things like that at app startup like we do in modern web-apps, which is why I'm bringing it up.

Do you see any issues with adding the ability to get a reference to the command to be executed? I am open to doing the work and sending a pull request if you don't have objections to the feature.

Tyrrrz commented 4 years ago

Do you see any issues with adding the ability to get a reference to the command to be executed?

Can you give an example of what that reference would look like? Or an example of how you would consume it in your Main() method to configure Serilog, assuming this feature was available??

Currently, CliFx has the concept of "schemas", i.e. CommandSchema, CommandOptionSchema and CommandParameterSchema which are essentially blueprints that tie the command types to their configuration. These types are internal and I would prefer to keep them that way as they are very volatile. I suppose you don't need all of that information, but just the type of the command that matches the given arguments?

nickdurcholz commented 4 years ago

Just getting at the type of the selected command would be sufficient for my needs. I could tag commands that require one configuration with a custom attribute and use that information to complete the setup. This would work:

    var services = new ServiceCollection();
    ...
    var serviceProvider = services.BuildServiceProvider();

    var cliApplication = new CliApplicationBuilder()
        .AddCommandsFromThisAssembly()
        .UseTypeActivator(new TypeActivator(serviceProvider))
        .UseDescription("AWS Instance Manager controls the state of the EC2 instance " +
                        "in response to SQS Messages and communicates the response " +
                        "back to the deployment scripts")
        .AllowDebugMode()
        .Build();

    var loggerConfig = new LoggerConfiguration()
        .ReadFrom.Configuration(config)
        .WriteTo.Async(sink => sink
            .File(Path.Combine(logDir, "InstanceManager.log"),
                rollingInterval: RollingInterval.Day,
                outputTemplate: "{Timestamp:o} {SourceContext} [{Level}] {Message}{NewLine}{Exception}",
                retainedFileCountLimit: 365)
        )
        .Enrich.FromLogContext();
    Type commandType = cliApplication.GetResolvedCommandType();
    if (commandType.GetCustomAttributes(typeof(NoLogToConsoleAttribute), true).Length == 0)
        loggerConfig.WriteTo.Console();
    Log.Logger = loggerConfig.CreateLogger();

    return cliApplication.RunAsync(args).AsTask();

I can also imagine a use case where knowing which parameters were provided would be beneficial, a --quiet switch for example. Providing a method to create a command instance that has all of the arguments processed and ready to execute would support this without exposing any internals of the framework. Example usage would be some thing like:

public partial class CliApplicationBuilder {
    public CliApplicationBuilder AddSetup(Action<ICommand> task) {
        ...
    }
}

public static class MyApplication {
    public static ValueTask<int> Main(string[] args) {
        var services = new ServiceCollection();
        ...

        var serviceProvider = services.BuildServiceProvider();

        return new CliApplicationBuilder()
            .AddCommandsFromThisAssembly()
            .UseTypeActivator(new TypeActivator(serviceProvider))
            .UseDescription("AWS Instance Manager controls the state of the EC2 instance " +
                            "in response to SQS Messages and communicates the response " +
                            "back to the deployment scripts")
            .AllowDebugMode()
            .AddSetup(InitializeLogging)
            .Build()
            .RunAsync(args)
            .AsTask();
    }

    public static ValueTask InitializeLogging(ICommand command) {
        var loggerConfig = new LoggerConfiguration()
            .ReadFrom.Configuration(config)
            .WriteTo.Async(sink => sink
                .File(Path.Combine(logDir, "InstanceManager.log"),
                    rollingInterval: RollingInterval.Day,
                    outputTemplate: "{Timestamp:o} {SourceContext} [{Level}] {Message}{NewLine}{Exception}",
                    retainedFileCountLimit: 365)
            )
            .Enrich.FromLogContext();
        if (command is MyApplicationCommandBase baseCommand && baseCommand.LogToStdOut)
            loggerConfig.WriteTo.Console();
        Log.Logger = loggerConfig.CreateLogger();
        return default(ValueTask);
    }
}

In this implementation, InitializeLogging would be called by CliApplication.RunAsync() before it calls instance.ExecuteAsync()

Tyrrrz commented 4 years ago

How about adding some type of hook like the following?

new CliApplicationBuilder()
            .AddCommandsFromThisAssembly()
            .UseTypeActivator(new TypeActivator(serviceProvider))
            .UseCommandProcessor((ICommand cmd) => /* do stuff with the command */)
            .Build();

That would expose the activated command with all of the properties populated.

Also, I just realized, a current workaround would probably be to use type activator something like this:

.UseTypeActivator(type =>
{
    if (type == typeof(FooBarCommand)
    {
         // ...
    }
});

Although that wouldn't let you access the bound arguments.

Another approach is to make a base command, i.e. LoggingCommandBase (or ILoggingCommand with C# 8 DIM) and configure logging in such way.

Depending on the context, inheritance might be much easier to deal with than a hook. Wdyt?

nickdurcholz commented 4 years ago

I had not noticed that overload for the type activator. I think that gets me what I need for now, so thanks for the suggestion.

I like the idea of the UseCommandProcessor hook. That seems like it has potential for allowing many use cases that weren't originally considered including this one.

Tyrrrz commented 4 years ago

I'm okay with introducing UseCommandProcessor (maybe need a better name though). If you want to give it a go, I'll create an issue for it and close this one (as it was opened primarily as a question).

rcdailey commented 3 years ago

This situation is pretty similar to the issue I opened #101. Did you make any progress on introducing a separate command processor?

nickdurcholz commented 3 years ago

The previous suggestion to put my initialization logic inside of the type activator worked well for me, so I didn't pursue this any further.