Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UseHost for WebApplicationBuilder (or maybe IHostApplicationBuilder) #2268

Open
djeikyb opened this issue Aug 23, 2023 · 4 comments
Open

Comments

@djeikyb
Copy link

djeikyb commented Aug 23, 2023

My team maintains a bunch of http apis using asp core. We've added a cli interface to each app because tho starting kestrel is the main case, there are several others, for example

  • configure backing services (eg, an auth provider)
  • run migrations
  • self-check: is all necessary configuration available?

Some are only invoked when running on a local dev machine, others are used in deploy pipelines.

The included System.CommandLine.Hosting.HostingExtensions has a UseHost for an IHostBuilder, but the host build object I have is an WebApplicationBuilder. Building the entire host is somewhat expensive, to be sure. But it's useful enough to be worth it, and the things we're running need most of the app anyway.

So I copied the UseHost function and swapped in the WebApplicationBuilder type. Mostly it's the same, except that the host is neither stopped nor started. I want the command handlers to decide whether or not to start the host! My RunWebApi handler does start the host, but my SelfCheck handler only needs the built host's ServicesCollection.

Does it make sense to add this extension method to the library? As I wrote this out, I realized that maybe IHostApplicationBuilder is a better type than WebApplicationBuilder, but the idea stands.

using System.CommandLine.Builder;
using System.CommandLine.Invocation;

namespace System.CommandLine.Hosting;

public static class HostingExtensions
{
    private const string ConfigurationDirectiveName = "config";
    public static CommandLineBuilder UseHost(this CommandLineBuilder builder,
        Func<string[], WebApplicationBuilder> hostBuilderFactory,
        Action<WebApplicationBuilder>? configureHost = null) =>
        builder.AddMiddleware(async (invocation, next) =>
        {
            var argsRemaining = invocation.ParseResult.UnparsedTokens.ToArray();
            var hostBuilder = hostBuilderFactory.Invoke(argsRemaining);
            hostBuilder.Host.Properties[typeof(InvocationContext)] = invocation;

            hostBuilder.Host.ConfigureHostConfiguration(config =>
            {
                config.AddCommandLineDirectives(invocation.ParseResult, ConfigurationDirectiveName);
            });
            hostBuilder.Host.ConfigureServices(services =>
            {
                services.AddSingleton(invocation);
                services.AddSingleton(invocation.BindingContext);
                services.AddSingleton(invocation.Console);
                services.AddTransient(_ => invocation.InvocationResult);
                services.AddTransient(_ => invocation.ParseResult);
            });
            hostBuilder.Host.UseInvocationLifetime(invocation);
            configureHost?.Invoke(hostBuilder);

            using var host = hostBuilder.Build();

            invocation.BindingContext.AddService(typeof(WebApplication), _ => host);

            // await host.StartAsync(); // no: let cli handlers decide if they want to start the host

            await next(invocation);

            // await host.StopAsync(); // don't stop what wasn't started
        });
@KalleOlaviNiemitalo
Copy link

IHostApplicationBuilder only exists starting from .NET 8.0 preview 6 (dotnet/runtime#86974, dotnet/aspnetcore#48775) so this would have to be in #if NET8_0_OR_GREATER. There is some precedent for public APIs varying by target framework, as the System.CommandLine.Hosting.InvocationLifetime constructor takes an IHostingEnvironment parameter on .NET Standard 2.0 but IHostEnvironment on .NET Standard 2.1.

The HostingExtensions implementation is quite different on the main branch, because the middleware APIs have been replaced with CliAction.

@djeikyb
Copy link
Author

djeikyb commented Aug 23, 2023

Ah ok yeah, then the sample here definitely isn't useful, except maybe to one or two other folks running the preview of this lib with asp. The sentiment stands, tho I haven't reviewed the main branch just yet to see if this issue still makes sense beyond the latest preview release.

@WeihanLi
Copy link

Maybe we could add a new extension like UseWebHost or something else for WebApplicationBuilder, use UseHost for the generic host

@fredrikhr
Copy link
Contributor

I have started work on PR #2450 where I propose new overloads to UseHost that accept HostApplicationBuilder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants