dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.49k stars 10.03k forks source link

Obsolete WebHostBuilder #20964

Open Tratcher opened 4 years ago

Tratcher commented 4 years ago

WebHostBuilder was replaced by HostBuilder in 3.0. In 5.0 we should consider obsoleting the old one.

analogrelay commented 4 years ago

To clarify even more, the idea here is to obsolete the types and public APIs on:

The obsolete message should include an aka.ms link that points to https://docs.microsoft.com/en-us/aspnet/core/fundamentals/host/generic-host?view=aspnetcore-3.1#settings-for-web-apps

davidfowl commented 4 years ago

We shouldn’t do this until we resolve the remaining gaps that people complain about:

Tratcher commented 4 years ago
  • Passing extra state to the startup class

Related issue: https://github.com/dotnet/aspnetcore/issues/19809

davidfowl commented 4 years ago

Also the IStartup replacement.

BrennanConroy commented 4 years ago

Lets start cleaning up areas before obsoleting this starting in preview6.

BrennanConroy commented 4 years ago

Example changes https://github.com/dotnet/aspnetcore/pull/23626

Make sure to dispose the IHost.

Common changes:

Before:

var host = new WebHostBuilder()
    .Configure(app =>
    {
        //...
    }).Build();

After:

using var host = new HostBuilder()
    .ConfigureWebHost(webHostBuilder =>
    {
        webHostBuilder
        .Configure(app =>
        {
            //...
        }
    }).Build();

TestServer: Before:

var builder = new WebHostBuilder();
var server = new TestServer(builder);

After:

using var host = new HostBuilder()
    // ...
    .UseTestServer()
    .Build();
var server = host.GetTestServer();
await host.StartAsync(); // new TestServer(builder); used to run Start, but we don't use that anymore
BrennanConroy commented 4 years ago

@mkArtakMSFT Can you find some folks to handle MVC? @HaoK Can you take care of Identity/Security?

Read previous comment for some helpers.

HaoK commented 4 years ago

Yup I can do identity/security

davidfowl commented 4 years ago

Just to be clear, we're not obsoleting anything in 5.0 😄. We haven't scheduled the linked items though I am supportive of updating the other projects.

HaoK commented 4 years ago

How bad is it if the host doesn't get disposed? There's a bunch of tests that rely on a helper method that creates the server, which if the helper has a using var host, the host gets disposed and the test code fails due to object disposed.

i.e.

   Task<TestServer> CreateServer() { 
      using var host = ... .Build(); 
      await host.StartAsync();
      return host.GetTestServer();
BrennanConroy commented 4 years ago

You'll get hangs because by default the console lifetime service is hooked up which waits for ctrl-c (or dispose) before closing.

I changed a bunch of helpers in middleware to return IHost instead of TestServer.

HaoK commented 4 years ago

Hangs? Everything seems to work fine if I just drop the using

HaoK commented 4 years ago

meaning the tests complete just fine without the using

BrennanConroy commented 4 years ago

Yeah, I'm not sure how VS handles that, but when running from command line you should see hangs.

HaoK commented 4 years ago

I just tried via dotnet test and it 'worked'. Does that mean its ok?

Tratcher commented 4 years ago

@BrennanConroy the other thing we should consider is having UseTestServer register its own lifetime to replace ConsoleLifetime. The implementation should be a no-op.

BrennanConroy commented 4 years ago

So, now there isn't risk of hangs for not disposing since we just merged the NoopHostLifetime. However, I'd still suggest disposing it if it's not too hard.

BrennanConroy commented 4 years ago

@mkArtakMSFT Can you find some folks to handle MVC?

Ping

Kahbazi commented 4 years ago

Is anybody working on MVC?

Tratcher commented 4 years ago

Is anybody working on MVC?

Not yet. MVC is a very large area with some complicated tests. I'd suggest splitting it into multiple PRs by project type (e.g. samples, tests, etc.) or feature area. Also, when you find something more unusual like WebApplicationFactory, only fix one test and submit a draft PR to make sure that's how they want it.

ghost commented 4 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

BrennanConroy commented 4 years ago
ghost commented 4 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost commented 3 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ghost commented 2 years ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.