Update libuv references in Kestrel docs #6184

guardrex · 2018-05-02T21:19:05Z

Update Updates

natemcmaster

A few points to clarify. Thanks for getting started on this.

natemcmaster · 2018-05-02T21:49:44Z

aspnetcore/fundamentals/servers/kestrel.md

+With the release of ASP.NET Core 2.1, Kestrel's default transport is no longer based on Libuv but instead based on managed sockets. This is a breaking change for ASP.NET Core 2.0 apps that call [WebHostBuilderLibuvExtensions.UseLibuv](/dotnet/api/microsoft.aspnetcore.hosting.webhostbuilderlibuvextensions.uselibuv) and depend on either of the following packages:
+
+* [Microsoft.AspNetCore.Server.Kestrel](https://www.nuget.org/packages/Microsoft.AspNetCore.Server.Kestrel/)
+* [Microsoft.AspNetCore.App](https://www.nuget.org/packages/Microsoft.AspNetCore.App/)


Since Microsoft.AspNetCore.App is new in 2.1, I might change the phrasing. This is only a breaking change for those using the Microsoft.AspNetCore.Server.Kestrel package directly without references to any metapackages.

Users upgrading from 2.0 to 2.1 shouldn't break unless they also take action to move from .All to .App. But in that case, they will break in a bunch of other ways too since .App doesn't have Redis, Sqlite, etc.

natemcmaster · 2018-05-02T21:53:24Z

aspnetcore/fundamentals/servers/kestrel.md

+```
+
+> [!NOTE]
+> The [Microsoft.AspNetCore.All](xref:fundamentals/metapackage) metapackage doesn't directly reference [Libuv](https://www.nuget.org/packages/Libuv/), but it does directly reference [Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv](https://www.nuget.org/packages/Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv/). The Libuv transport package is transitively referenced when the `Microsoft.AspNetCore.All` package is referenced. No action is required by the developer when the `Microsoft.AspNetCore.All` metapackage is used and `UseLibuv` is called in the app. Note that the ASP.NET Core 2.1 or later templates reference the [Microsoft.AspNetCore.App](https://www.nuget.org/packages/Microsoft.AspNetCore.App/) metapackage by default. The use of the `Microsoft.AspNetCore.All` metapackage isn't recommended for ASP.NET Core 2.1 or later apps.


This is pretty wordy. I think your main point here is that the previous step is unnecessary in some cases, right? If so, consider shortening to

No additional reference is required when using the Microsoft.AspNetCore.All metapackage, which will pull in Libuv transitively. For more details on this metapackage, see {cref}

guardrex · 2018-05-02T22:21:46Z

@natemcmaster ok ... see how that shorter version looks.

natemcmaster · 2018-05-02T23:28:19Z

aspnetcore/fundamentals/servers/kestrel.md

@@ -75,6 +82,24 @@ ASP.NET Core project templates use Kestrel by default. In *Program.cs*, the temp

 [!code-csharp[](kestrel/samples/2.x/Program.cs?name=snippet_DefaultBuilder&highlight=7)]

+::: moniker range=">= aspnetcore-2.1"
+**Default transport changes from Libuv to managed sockets (ASP.NET Core 2.1 or later)**


Reviewing this in context of the rest of the document now...

I think this new block of text belongs lower in the doc. Transport configuration deserves its own section, maybe after "Endpoint configuration" and before "URL prefixes". The breaking change from Libuv to Sockets should be one of the things mentioned in that section, but I don't expect this breaking change to affect many people. We're hoping that in most cases no action will be required.

(I'm not expecting you to write this, but) it would also be good to have the transport configuration section explain why you might want to use sockets vs libuv. That can be punted for now, but it's worth addressing because changing the transport off default isn't a common scenario. We should be able to justify why it's ever needed.

guardrex · 2018-05-03T01:04:35Z

@natemcmaster Sounds good. I moved the section down on the last commit.

it would also be good to have the transport configuration section explain why you might want to use sockets vs libuv

Can you or one of the engineers provide the content? ... even if only a list of pros/cons of each or a single list of why sockets are better. It would be good for us to get enough coverage in place for now to close this issue and not open a new one (if possible).

guardrex · 2018-05-03T16:55:11Z

Are the motivations for sockets over Libuv you laid out in aspnet/KestrelHttpServer#2360 (comment) applicable for the community?

Motivations:

broad platform support. Wherever managed sockets are supported, Kestrel should work. Doesn't require recompiling libuv to bring up a new OS.
source-build. (https://github.com/dotnet/source-build). In current form, Kestrel packages can't be produced in a source-build environment
ecosystem. Helps to drive improvement to the .NET sockets implementation, which benefits anyone else using managed sockets.

If so, I can draft some text that we can work forward.

halter73 · 2018-05-03T19:29:30Z

@guardrex Those are the primary motivations to default to the socket transport. I don't think we should call too much attention to configuring Kestrel transports in the docs though. I expect that the difference between transports won't be noticeable to the vast majority of developers.

halter73 · 2018-05-03T19:31:20Z

aspnetcore/fundamentals/servers/kestrel.md

+                  Version="2.1.0" />
+```
+
+No action is required when the [Microsoft.AspNetCore.All](xref:fundamentals/metapackage) metapackage is used and `UseLibuv` is called because the Libuv transport package is transitively referenced by the `Microsoft.AspNetCore.All` metapackage. Note that the ASP.NET Core 2.1 or later templates reference the [Microsoft.AspNetCore.App](https://www.nuget.org/packages/Microsoft.AspNetCore.App/) metapackage by default. The use of the `Microsoft.AspNetCore.All` metapackage isn't recommended for ASP.NET Core 2.1 or later apps.


I didn't realize that Microsoft.AspNetCore.All was deprecated. Maybe we shouldn't mention it at all and just tell people to reference Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv directly.

I believe they're phrasing it more along the lines of "not recommended" and "subject to future removal" at this time. That's why I slanted the language in this way (i.e., not use the word "depreciated" there). *they = @DamianEdwards in Standup remarks.

This is a good point. I'm not sure this is a recommendation we actually need to make yet. But even if we do, this seems like information that belongs on the metapackage doc, not here.

natemcmaster · 2018-05-03T19:34:42Z

I would leave out the reasons for changing the default. IMO the only important reason document transport config at all is for compatibility. If users upgrade to 2.1 but rely on some libuv-only features, they will need to switch their transport to Libuv. At the moment, we're only aware of a few, rarely used features of libuv that are not available in sockets. +1 to what @halter73 said - we expect transport configuration to be an uncommon option and 99% of users will be fine with the default.

halter73 · 2018-05-03T19:36:31Z

aspnetcore/fundamentals/servers/kestrel.md

 Kestrel is a cross-platform [web server for ASP.NET Core](xref:fundamentals/servers/index) based on [libuv](https://github.com/libuv/libuv), a cross-platform asynchronous I/O library. Kestrel is the web server that's included by default in ASP.NET Core project templates.
+::: moniker-end
+::: moniker range=">= aspnetcore-2.1"
+Kestrel is a cross-platform [web server for ASP.NET Core](xref:fundamentals/servers/index) based on managed sockets (`Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets`), a cross-platform asynchronous I/O library. In versions of ASP.NET Core prior to 2.1, Kestrel is based on [libuv](https://github.com/libuv/libuv).


I think we should leave discussion of libuv and managed sockets transports out of the intro. Mentioning libuv before wasn't so bad because it was short, but now it seems overly technical for the intro paragraph. This should be irrelevant to the vast majority of ASP.NET developers.

ok ... I'll address that on the next commit.

I don't know how detailed we want this paragraph to be, but I think something like "Kestrel is a cross-platform web server for ASP.NET Core that's included by default in ASP.NET Core project templates" is enough.

guardrex · 2018-05-03T19:38:07Z

RE: The new section

Are we all on the same page?

Leave this in it's new section
Don't sweat more content than what I have

halter73 · 2018-05-03T19:39:26Z

aspnetcore/fundamentals/servers/index.md

 * [Kestrel](xref:fundamentals/servers/kestrel) is a cross-platform HTTP server based on [libuv](https://github.com/libuv/libuv), a cross-platform asynchronous I/O library.
+::: moniker-end
+::: moniker range=">= aspnetcore-2.1"
+* [Kestrel](xref:fundamentals/servers/kestrel) is a cross-platform HTTP server based on managed sockets (`Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets`), a cross-platform asynchronous I/O library. In versions of ASP.NET Core prior to 2.1, Kestrel is based on [libuv](https://github.com/libuv/libuv).


I also don't think discussion of the transport belongs here. If we must include it, I wouldn't call Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets a cross-platform asynchronous I/O library. That's what libuv is, but the sockets transport is more limited. It's just a socket library that implements Kestrel's transport abstraction.

Sure, I can address that on the next commit.

I know we go into this in detail in the kestrel-specific doc, but I think it's still worth mentioning libuv is still an option for 2.1 here.

That, or just remove the transport discussion from the main server doc.

Yes, I think we should remove it. If I try to add that sentiment, it becomes perhaps a bit too complicated ...

Kestrel is a cross-platform HTTP server based on managed sockets (Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets), a cross-platform asynchronous I/O library. In versions of ASP.NET Core prior to 2.1, Kestrel is based on libuv, which remains a viable alternative for 2.1 or later apps.

We can just go with this here ...

Kestrel is a cross-platform HTTP server based on managed sockets (Microsoft.AspNetCore.Server.Kestrel.Transport.Sockets), a cross-platform asynchronous I/O library.

halter73 · 2018-05-03T19:41:10Z

@guardrex I think the content you've added already is enough. I would just keep the old intros more-or-less with the libuv stuff removed.

guardrex · 2018-05-03T20:10:53Z

@Rick-Anderson Would you take a look and see if u feel I should cut/move content? ... particularly the last paragraph.

Rick-Anderson · 2018-05-03T20:18:03Z

@guardrex I'd go with what @halter73 suggests.

guardrex · 2018-05-03T20:19:11Z

ty @Rick-Anderson

@halter73 I think the last commit addresses your feedback ... let me know if not, and I'll square it away.

@natemcmaster I can move that bit (or another author will compose it) as soon as we get App package coverage (possibly migration-to-2.1 coverage, too) in place.

guardrex · 2018-05-09T20:19:06Z

How are we on engineering approval here? Are we good?

I can move that last paragraph regarding the metapackages situation to the new "App" package content being added by #6069 as soon as that merges. However, I think we should push this forward as-is, and I'll open a new issue to move the paragraph shortly.

halter73

I left one more comment because I don't like how the new language implies that libuv is no longer supported in 2.1, but overall it looks good.

halter73 · 2018-05-09T22:28:38Z

aspnetcore/fundamentals/servers/kestrel.md

+```xml
+<PackageReference Include="Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv" 
+                  Version="2.1.0" />
+```


One more thing. Can we have a one-line snippet like this, but for the actual call to UseLibuv()?

guardrex · 2018-05-09T22:56:44Z

@halter73

one-line snippet

How about a bit more context? ... I've learned the hard way from readers that they really don't like one liners all that much ... they get lost and don't know where to place the line. See the last commit.

[EDIT] I added two options. Show one? If so, which one? Or show both? ... or do u still want to go with a one-liner?

…spnet/Docs into guardrex/kestrel-libuv-updates

halter73 · 2018-05-10T03:47:17Z

aspnetcore/fundamentals/servers/index.md

@@ -20,14 +20,13 @@ An ASP.NET Core app runs with an in-process HTTP server implementation. The serv
 ASP.NET Core ships two server implementations:

 * [Kestrel](xref:fundamentals/servers/kestrel) is a cross-platform HTTP server based on [libuv](https://github.com/libuv/libuv), a cross-platform asynchronous I/O library.


Is this a 1.x section? If not, I'd just drop everything after and including "based on". Maybe you could mention it's the default server, but that's covered almost immediately after this already.

halter73 · 2018-05-10T03:48:22Z

aspnetcore/fundamentals/servers/kestrel.md

+            WebHost.CreateDefaultBuilder(args)
+                .UseLibuv()
+                .UseStartup<Startup>();
+    }


This looks great. I think the first version is enough since that's what most people see in their templates these days.

guardrex · 2018-05-10T12:03:50Z

ty @halter73 ... the first one was just exhaustion taking over. Updates made.

scottaddie · 2018-05-10T15:28:18Z

aspnetcore/fundamentals/hosting.md


 ```csharp
 var host = new WebHostBuilder()
    .CaptureStartupErrors(true)
    ...
 ```

---
+* * *


The tab specification doc calls for 3 consecutive dashes. Let's stick with that.

I just saw these start to change somewhere ... I thought they were going for a new format ... along with the four hash tabs.

Where did I see this coming in? I'll research and see where I got this.

It was this one ... #5878

These were changed in a lot of spots (but not everywhere).

scottaddie · 2018-05-10T15:28:35Z

aspnetcore/fundamentals/hosting.md


 ```csharp
 var host = new WebHostBuilder()
    .UseContentRoot("c:\\mywebsite")
    ...
 ```

---
+* * *


Convert to 3 dashes.

scottaddie · 2018-05-10T15:31:53Z

aspnetcore/fundamentals/servers/kestrel.md

+    }
+    ```
+
+No action is required when the [Microsoft.AspNetCore.All](xref:fundamentals/metapackage) metapackage is used and `UseLibuv` is called because the Libuv transport package is transitively referenced by the `Microsoft.AspNetCore.All` metapackage. Note that the ASP.NET Core 2.1 or later templates reference the [Microsoft.AspNetCore.App](https://www.nuget.org/packages/Microsoft.AspNetCore.App/) metapackage by default. The use of the `Microsoft.AspNetCore.All` metapackage isn't recommended for ASP.NET Core 2.1 or later apps.


Since this section only displays for ASP.NET Core 2.1+, we shouldn't reference the older ".All" metapackage. That will really slim down this paragraph.

I think it would be best in a migration topic, but I don't see TO-2.1 content in the migration area.

This para has taken a lot of heat. 😄 lol I'd prefer to cut it at 3.0; but since everyone else dislikes it, I'll cut it.

Update libuv references in Kestrel docs

5cf79ff

Update Updates

guardrex requested review from halter73, Tratcher and natemcmaster May 2, 2018 21:19

natemcmaster reviewed May 2, 2018

View reviewed changes

React to feedback

a993c84

natemcmaster reviewed May 2, 2018

View reviewed changes

Move content into a section

80f6801

halter73 reviewed May 3, 2018

View reviewed changes

React to feedback

7a4e92b

guardrex mentioned this pull request May 3, 2018

Add Microsoft.AspNetCore.App to dotnet docs repo #5937

Closed

halter73 approved these changes May 9, 2018

View reviewed changes

halter73 reviewed May 9, 2018

View reviewed changes

React to feedback

fce489c

guardrex added 2 commits May 9, 2018 18:09

Remove 2.1 include

ded0d1e

Merge branch 'master' into guardrex/kestrel-libuv-updates

c2db7f5

guardrex added 2 commits May 9, 2018 19:00

Show both

a64c358

Merge branch 'guardrex/kestrel-libuv-updates' of https://github.com/a…

3c178c0

…spnet/Docs into guardrex/kestrel-libuv-updates

halter73 reviewed May 10, 2018

View reviewed changes

Updates

79edfd8

guardrex requested a review from scottaddie May 10, 2018 12:04

Repair code block+highlights

e547864

scottaddie reviewed May 10, 2018

View reviewed changes

Reverting tab controls to official guidance (AFAIK)

1213555

guardrex merged commit 0f0cd06 into master May 10, 2018

guardrex deleted the guardrex/kestrel-libuv-updates branch May 10, 2018 21:12

		@@ -20,14 +20,13 @@ An ASP.NET Core app runs with an in-process HTTP server implementation. The serv
		ASP.NET Core ships two server implementations:

		* [Kestrel](xref:fundamentals/servers/kestrel) is a cross-platform HTTP server based on [libuv](https://github.com/libuv/libuv), a cross-platform asynchronous I/O library.

Update libuv references in Kestrel docs #6184

Update libuv references in Kestrel docs #6184

Conversation

guardrex commented May 2, 2018

natemcmaster left a comment

Choose a reason for hiding this comment

natemcmaster May 2, 2018 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

guardrex commented May 2, 2018

Choose a reason for hiding this comment

guardrex commented May 3, 2018

guardrex commented May 3, 2018

halter73 commented May 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

natemcmaster commented May 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guardrex commented May 3, 2018

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

guardrex May 9, 2018 • edited Loading

Choose a reason for hiding this comment

halter73 commented May 3, 2018 • edited Loading

guardrex commented May 3, 2018

Rick-Anderson commented May 3, 2018

guardrex commented May 3, 2018 • edited Loading

guardrex commented May 9, 2018

halter73 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guardrex commented May 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guardrex commented May 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guardrex May 10, 2018 • 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

natemcmaster May 2, 2018 •

edited

Loading

guardrex May 9, 2018 •

edited

Loading

halter73 commented May 3, 2018 •

edited

Loading

guardrex commented May 3, 2018 •

edited

Loading

guardrex commented May 9, 2018 •

edited

Loading

guardrex May 10, 2018 •

edited

Loading