WIP: Simplify hosting configuration with some extension methods #991

davidfowl · 2017-03-27T05:35:46Z

Added WaitForShutdown method to IWebHost
Added Run and RunApplication to IWebHostBuilder to Build and
Start the application in a single method.

After a discussion about what could be done to further simplify some of the startup logic with the WebHostBuilder a few things came up. These were some of the methods that we could add to make the startup logic more approachable while not completely diverging from the original API.

Further simplification requires picking kestrel to to be the default server but that won't happen as part of this PR and requires further discussion.

/cc @glennc @Tratcher @DamianEdwards

- Added WaitForShutdown method to IWebHost - Added Run and RunWith to IWebHostBuilder to Build and Start the application in a single method.

davidfowl · 2017-03-27T05:37:45Z

src/Microsoft.AspNetCore.Hosting/WebHostBuilderExtensions.cs

+        /// </summary>
+        /// <param name="hostBuilder">The <see cref="IWebHostBuilder"/> to run.</param>
+        /// <param name="configure">The delegate that configures the <see cref="IApplicationBuilder"/>.</param>
+        public static IWebHost RunWith(this IWebHostBuilder hostBuilder, Action<IApplicationBuilder> configure)


This name isn't really good. RunAndConfigure? ConfigureAndRun? RunWithConfiguration?

Once an IApplicationBuilder is needed it's probably alright to call Configure and Build separately. If another Run API then maybe one that takes Run(handler, port) to avoid the extra UseUrls. Also like the idea of having Kestrel as a default.

Maybe, I'd like to play around with it a little before dumping it. Needing middleware isn't an advanced scenario, it's what you do after you get hello world working.

As for the port, overload, the assumption would be that it binds to * and not localhost (we've had these debates over and over). I'll add it and see what the feedback is.

I don't love these methods. I added port as the first argument but multiple arguments with one being a delegate is always ugly IMO.

foo.Run(app => { }, 8080); foo.Run(8080, app=> { }); foo.Listen(8080).Run(app => { });

lutzroeder · 2017-03-27T05:56:08Z

src/Microsoft.AspNetCore.Hosting/WebHostExtensions.cs

+            var hostingEnvironment = host.Services.GetService<IHostingEnvironment>();
+            var applicationLifetime = host.Services.GetService<IApplicationLifetime>();
+
+            Console.WriteLine($"Hosting environment: {hostingEnvironment.EnvironmentName}");


Can this be implemented without always writing to Console?

There's a whole discussion on this too. I think the default should use the console. Having an options to turn it off would be fine though. See #979

davidfowl · 2017-03-27T06:03:05Z

src/Microsoft.AspNetCore.Hosting/WebHostExtensions.cs

+        /// <param name="token">The token to trigger shutdown.</param>
+        public static void WaitForShutdown(this IWebHost host, CancellationToken token)
+        {
+            host.WaitForShutdown(token, shutdownMessage: null);


Should this dispose the host? Part of the API is about simplicity but it's also generally useful outside of that use case.

lutzroeder · 2017-03-27T06:28:50Z

src/Microsoft.AspNetCore.Hosting/WebHostBuilderExtensions.cs

+        /// <param name="hostBuilder">The <see cref="IWebHostBuilder"/> to run.</param>
+        /// <param name="port">The port to bind to.</param>
+        /// <param name="handler">A delegate that handles the request.</param>
+        public static IWebHost Run(this IWebHostBuilder hostBuilder, int port, RequestDelegate handler)


Isn't there some FxCop rule that says keep adding new parameters at the end of the method if name stays the same (agree having the port first makes more sense otherwise).

We don't use FXCop 😄

Tratcher · 2017-03-27T15:57:48Z

samples/SampleStartups/StartupHelloWorld.cs

-                await context.Response.WriteAsync("Hello World!");
-            });
+            var host = new WebHostBuilder()
+                .Run(async context =>


BuildAndStart?

Too ugly. It's really BuildRunStart so no...

Tratcher · 2017-03-27T15:59:41Z

samples/SampleStartups/StartupHelloWorld.cs

+        public static void MainWithPort()
+        {
+            var host = new WebHostBuilder()
+                .Run(8080, async context =>


We've avoided specifying just a port because of the ambiguity over what the IP/Host is. Localhost? */Any? Loopack? IPv6Loopback? There's never been agreement for what the default host/ip should be.

I also don't see this extension being used outside of demoware, no real app is this simple. I'd rather have the RunApplication variant as Run.

We've avoided specifying just a port because of the ambiguity over what the IP/Host is. Localhost? */Any? Loopack? IPv6Loopback? There's never been agreement for what the default host/ip should be.

I know, I was there, I'm forcing the discussion by picking Any.

I also don't see this extension being used outside of demoware, no real app is this simple. I'd rather have the RunApplication variant as Run.

Agreed. Or very simple applications.

Yea this is dangerous. to me it reads like "bind to any nic, on any IP I have". That's going to be a nasty surprise for someone. Be specific here.

I've added a required hostname.

url instead of port is probably ok. UseUrls is a little bit more but having Run(...) or some similar method just take the handler instead of calling Configure is a great simplification.

Tratcher · 2017-03-27T16:09:55Z

src/Microsoft.AspNetCore.Hosting/WebHostExtensions.cs

+        /// <param name="token">The token to trigger shutdown.</param>
+        public static void Run(this IWebHost host, CancellationToken token)
+        {
+            host.Run(token, shutdownMessage: null);


No default shutdown message?

This hasn't changed.

Tratcher · 2017-03-27T16:12:51Z

src/Microsoft.AspNetCore.Hosting/WebHostBuilderExtensions.cs

+        /// <param name="handler">A delegate that handles the request.</param>
+        public static IWebHost Run(this IWebHostBuilder hostBuilder, int port, RequestDelegate handler)
+        {
+            var host = hostBuilder.UseUrls($"http://*:{port}/")


Yeah, @blowdart has killed this before. Too many localhost test servers being publicly exposed.

No no no no no. Don't make me cry.

I think it has to be this way for compat with other frameworks.

golang:

For TCP and UDP, the syntax of laddr is "host:port", like "127.0.0.1:8080". If host is omitted, as in ":8080", Listen listens on all available interfaces instead of just the interface with the given host address.

nodejs

Begin accepting connections on the specified port and hostname. If the hostname is omitted, the server will accept connections directed to any IPv4 address (INADDR_ANY).

As a compromise, we could require both host and port.

We already require host and port (and scheme). Unless you're going to accept them a separate parameters I don't see this extension adding much value over UseUrls.

You don't but others do. Everything here is doable already this isn't about functionality. If it was we wouldn't make new APIs. It's about ease of use, API design, the right nomenclature and common parameters used to Start, Listen, Run a simple server.

Tend to agree. Taking url instead of just the port makes sense given it's consistent overall.

Tratcher · 2017-03-27T16:16:47Z

src/Microsoft.AspNetCore.Hosting/WebHostBuilderExtensions.cs

+        /// </summary>
+        /// <param name="hostBuilder">The <see cref="IWebHostBuilder"/> to run.</param>
+        /// <param name="handler">A delegate that handles the request.</param>
+        public static IWebHost Run(this IWebHostBuilder hostBuilder, RequestDelegate handler)


Why aren't these extensions named Start? the IWebHost.Run extensions all block but these call Start and return.

davidfowl · 2017-04-04T08:27:00Z

Will revisit.

DamianEdwards · 2017-04-06T05:39:57Z

We're looking at doing something in the meta-package: aspnet/MetaPackages#24

Simplify hosting configuration with some extension methods

e5942d3

- Added WaitForShutdown method to IWebHost - Added Run and RunWith to IWebHostBuilder to Build and Start the application in a single method.

dnfclas added the cla-already-signed label Mar 27, 2017

davidfowl changed the title ~~Simplify hosting configuration with some extension methods~~ WIP: Simplify hosting configuration with some extension methods Mar 27, 2017

davidfowl commented Mar 27, 2017

View reviewed changes

lutzroeder reviewed Mar 27, 2017

View reviewed changes

Rename RunWith to RunApplication

66a6764

davidfowl commented Mar 27, 2017

View reviewed changes

Added port overload

312fced

lutzroeder reviewed Mar 27, 2017

View reviewed changes

davidfowl requested review from glennc and Tratcher March 27, 2017 15:51

Tratcher suggested changes Mar 27, 2017

View reviewed changes

Added hostname parameter to avoid ambiguity

acd70f7

davidfowl closed this Apr 4, 2017

DamianEdwards mentioned this pull request Apr 6, 2017

Add new WebHost static API to Microsoft.AspNetCore meta-package with opinionated defaults aspnet/MetaPackages#24

Closed

davidfowl deleted the davidfowl/simplify-bootup branch April 18, 2017 07:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Simplify hosting configuration with some extension methods #991

WIP: Simplify hosting configuration with some extension methods #991

davidfowl commented Mar 27, 2017 •

edited

Loading

davidfowl Mar 27, 2017

lutzroeder Mar 27, 2017

davidfowl Mar 27, 2017

davidfowl Mar 27, 2017

davidfowl Mar 27, 2017

lutzroeder Mar 27, 2017 •

edited

Loading

davidfowl Mar 27, 2017 •

edited

Loading

davidfowl Mar 27, 2017

lutzroeder Mar 27, 2017

davidfowl Mar 27, 2017

Tratcher Mar 27, 2017 •

edited

Loading

davidfowl Mar 27, 2017 •

edited

Loading

Tratcher Mar 27, 2017

Tratcher Mar 27, 2017

davidfowl Mar 27, 2017

blowdart Mar 27, 2017

davidfowl Mar 27, 2017

lutzroeder Mar 27, 2017

Tratcher Mar 27, 2017

davidfowl Mar 27, 2017

Tratcher Mar 27, 2017

blowdart Mar 27, 2017

davidfowl Mar 27, 2017

Tratcher Mar 27, 2017

davidfowl Mar 27, 2017

lutzroeder Mar 27, 2017

Tratcher Mar 27, 2017

davidfowl commented Apr 4, 2017

DamianEdwards commented Apr 6, 2017

WIP: Simplify hosting configuration with some extension methods #991

WIP: Simplify hosting configuration with some extension methods #991

Conversation

davidfowl commented Mar 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lutzroeder Mar 27, 2017 • edited Loading

Choose a reason for hiding this comment

davidfowl Mar 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tratcher Mar 27, 2017 • edited Loading

Choose a reason for hiding this comment

davidfowl Mar 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davidfowl commented Apr 4, 2017

DamianEdwards commented Apr 6, 2017

davidfowl commented Mar 27, 2017 •

edited

Loading

lutzroeder Mar 27, 2017 •

edited

Loading

davidfowl Mar 27, 2017 •

edited

Loading

Tratcher Mar 27, 2017 •

edited

Loading

davidfowl Mar 27, 2017 •

edited

Loading