aspnet / Hosting

[Archived] Code for hosting and starting up an ASP.NET Core application. Project moved to https://github.com/aspnet/Extensions and https://github.com/aspnet/AspNetCore
Apache License 2.0
552 stars 312 forks source link

Design question: How should other application models configure the generic Host #1279

Closed jdom closed 6 years ago

jdom commented 6 years ago

Hi, we in Orleans are going to use the new generic Microsoft.Extensions.Hosting.HostBuilder that is going to be released in 2.1. Nevertheless, we are wondering what is the expectation of other non-ASPNET application models such as ours with regards to extension methods that configure the host.

For example in the currently released version of ASP.NET Core (2.0), since it was thought out as the only hosted application in the container and host, every extension method extends IServiceCollection (with the exception of IApplicationBuilder for the configuration that can be safely done after the service provider was built) and their names do not clearly identify that it is modifying how ASP.NET works. This is completely fine for cross cutting concerns such as logging, but can be ambiguous for ASP.NET specific config once co-hosting is a thing.

So look at this code for example:

var host = new HostBuilder()
    .ConfigureAppConfiguration((hostContext, config) => config.AddJsonFile("appsettings.json", optional: true))
    .ConfigureServices((hostContext, services) =>
    {
        services.AddDbContext<ApplicationDbContext>(options =>
            options.UseSqlServer(hostContext.Configuration.GetConnectionString("DefaultConnection")));

        services.AddIdentity<ApplicationUser, IdentityRole>()
            .AddEntityFrameworkStores<ApplicationDbContext>()
            .AddDefaultTokenProviders();

        services.AddMvc();

        services
            .AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme)
                .AddCookie(options => options.EventsType = typeof(CustomCookieAuthenticationEvents));
        services.AddScoped<CustomCookieAuthenticationEvents>();

        // this is Orleans-specific configuration, but would you be able to tell without this comment?
        services
            .Configure<MessagingOptions>(o => o.ResponseTimeout = TimeSpan.FromSeconds(20))
            .UseAzureTableMembership(o => o.ConnectionString = "xxx")
            .AddCosmosDbStorage("myStorageProvider1", o => { })
            .AddCosmosDbStorage("myStorageProvider2", o => { })
            .AddEventHubStream("myStreamProvider1", ob => 
                ob.Configure(hostContext.Configuration.GetSection("myStreamProvider1"))
                  .Configure(o => o.PrefetchCount = 20))

        // same ambiguity occurs when configuring framework Foo
    })
    .AddApplicationPartsFromReferences(typeof(IHelloWorldGrain).Assembly) // this one is Orleans-specific too, directly on the HostBuilder, not IServiecCollection
    .UseIISIntegration() // this only applies to ASP.NET
    .ConfigureWebHost((hostContext, app) =>
    {
        app.Run((context) => context.Response.WriteAsync("Hello World!"));
    })
    .UseConsoleLifetime()
    .Build();

You can see that this is co-hosting Orleans and ASP.NET, but it is really not very clear which calls configure Orleans vs ASP.NET.

Is this how we intend to go or have you thought about a way to break this ambiguity? We are considering having all extension methods to hang from an inner Orleans host builder (kind of how you use IApplicationBuilder passed in to ConfigureWebHost, but that runs and ends up configuring the IServiceCollection instead of running after the container is built). If we go this route (with an inner builder), does it mean we shouldn't provide extension methods on IServiceCollection then or do we provide both? Providing both is more flexible but can bring this ambiguity back. It also means an explosion of extension methods, since we need to provide one for the builder and a similar one for IServiceCollection.

We would appreciate any guidance on how we should approach this.

jdom commented 6 years ago

/cc @glennc @davidfowl @Tratcher @muratg @cwe1ss @jason-bragg @ReubenBond

cwe1ss commented 6 years ago

In terms of IServiceCollection it seems like many frameworks/libraries use a IXBuilder approach that encapsulates an IServiceCollection. So you have a IServiceCollection.AddMvc that returns a IMvcBuilder - and all the framework-specific configurations are extensions on this interface.

So you would have a IServiceCollection.AddOrleans that returns a IOrleansBuilder. This would then have methods like AddCosmosDbStorage...

This doesn’t solve the problem of how IHostBuilder extensions should be handled though.

Tratcher commented 6 years ago

Note ASP.NET Core isn't moving to the generic host model until at least 3.0 as it will require several breaking changes for us. ConfigureWebHost is the only prototype we have for what that may look like.

For services, what if we did the following instead of sub builders?

var host = new HostBuilder()
    .ConfigureAppConfiguration((hostContext, config) => config.AddJsonFile("appsettings.json", optional: true))
    .ConfigureOrleansServices((hostContext, services) =>
    {
        // this is Orleans-specific configuration
        services
            .Configure<MessagingOptions>(o => o.ResponseTimeout = TimeSpan.FromSeconds(20))
            .UseAzureTableMembership(o => o.ConnectionString = "xxx")
            .AddCosmosDbStorage("myStorageProvider1", o => { })
            .AddCosmosDbStorage("myStorageProvider2", o => { })
            .AddEventHubStream("myStreamProvider1", ob => 
                ob.Configure(hostContext.Configuration.GetSection("myStreamProvider1"))
                  .Configure(o => o.PrefetchCount = 20))
    })
    .AddApplicationPartsFromReferences(typeof(IHelloWorldGrain).Assembly) // this one is Orleans-specific too, directly on the HostBuilder, not IServiecCollection
    .UseIISIntegration() // this only applies to ASP.NET
    .ConfigureWebHostServices((hostContext, services) =>
    {
        services.AddDbContext<ApplicationDbContext>(options =>
            options.UseSqlServer(hostContext.Configuration.GetConnectionString("DefaultConnection")));

        services.AddIdentity<ApplicationUser, IdentityRole>()
            .AddEntityFrameworkStores<ApplicationDbContext>()
            .AddDefaultTokenProviders();

        services.AddMvc();

        services
            .AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme)
                .AddCookie(options => options.EventsType = typeof(CustomCookieAuthenticationEvents));
        services.AddScoped<CustomCookieAuthenticationEvents>();
    })
    .ConfigureWebHost((hostContext, app) =>
    {
        app.Run((context) => context.Response.WriteAsync("Hello World!"));
    })
    .UseConsoleLifetime()
    .Build();

You wouldn't get a separate container, but at least you could group the web specific services together visually. However this does not improve the intellesense any.

jdom commented 6 years ago

Thanks for the response. But yes, that last suggestion would only be guidance to end users, but still lacks any kind of actual help from the framework to distinguish between the two. Perhaps a sub builder that holds an IServiceCollection is the most appropriate. But if we go that route, do we create extension methods on just the IOrleansBuilder or do we duplicate everything so users can also use IServiceCollection if they don't care about this cross pollination (or are not even co-hosting)? My main concern if we don't expose the extension methods for IServiceCollection at all, is that it is sometimes useful to query IServiceCollection and then call some extension method based on the state (granted, this is mostly for infrastructure level code, and maybe only because I don't know better)

muratg commented 6 years ago

cc @davidfowl

aspnet-hello commented 6 years ago

We periodically close 'discussion' issues that have not been updated in a long period of time.

We apologize if this causes any inconvenience. We ask that if you are still encountering an issue, please log a new issue with updated information and we will investigate.