Skip to content

Conversation

@Silvenga
Copy link
Contributor

@Silvenga Silvenga commented Feb 14, 2017

This pull request adds support for the Kestrel server.

Kestrel is a cross-platform web server for ASP.NET Core based on libuv, a cross-platform asynchronous I/O library. Kestrel is the web server that is included by default in ASP.NET Core new project templates.

The move was easier than expected, most of the changes are the new packages, the preview server is virtually the same.

A basic pro/cons list (some are IMHO):

Gains

  • Trivial support for HTTPS.
  • Cross platform.
  • No longer uses http.sys.
    • No more issues with url reservations under Windows (I have very strong feelings for urlacl 😒).
    • Not hostname verification.
  • Kestrel will replace Katana.
  • Easier support for dotNetCore.
  • WebSockets! <- makes WIP: Adds basic support for LiveReload when watching/previewing #420 a bit more possible.

Losses

  • No longer uses http.sys.
  • No support for port sharing with IIS (did anyone use this anyway?)
  • Complicated short term support:
    • Native nuget packages are not supported out-of-box < VS2017 projects (custom includes required). This is mitigated by a move to dotNetCore (planned?).
    • Only supports Windows x86 with the default build. This is also mitigated by either upgrading to VS2017 or dotNetCore.

Considerations

  • Large functional move with little benefit in the majority of cases. I'm primarily concerned with what is unknown with Kestrel - not much community usage yet + not much personal experience.
  • Reduces portability for now, but Wyam doesn't support Mono anyway, so moot?

@Silvenga Silvenga changed the title Moves Preview Server to Use Kestrel over HttpListener Moves Preview Server to Use Kestrel Feb 14, 2017
<None Include="App.config" />
<None Include="packages.config" />
</ItemGroup>
<ItemGroup>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the hacky include. This assumes a x86 compatible build.

@daveaglick
Copy link
Member

Totally on board with this. In fact, there's an open issue #385 to move the preview server to an entirely separate library so what I'll probably do is merge this in and then split it out. Then the work in #420 can move to the new Wyam.Server library.

@Silvenga
Copy link
Contributor Author

Sweet!

I would propose a merge of this and #420 before the break out so that #420 doesn't need to be rebuilt. #420 essentially works, just not under Windows 7, worst case the changes are purely additive.

@daveaglick
Copy link
Member

That sounds like a plan - no reason to make you deal with the split while trying to work out the rebase. We'll merge this one as-is, take another look at LiveReload, hopefully be able to merge that one now, and then I'll split them out. That'll be a better verification anyway since we'll know it worked pre-split.

Hopefully I'll get a chance to merge this one later tonight or early tomorrow.

@daveaglick
Copy link
Member

Worked fine on my test merge, so I'll pull this in and we'll deal with any other problems as they arise.

@daveaglick daveaglick merged commit 9649ae3 into statiqdev:develop Feb 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants