Skip to content
This repository has been archived by the owner on Dec 19, 2018. It is now read-only.

Update Hosting API and TestServer API #525

Closed
JunTaoLuo opened this issue Dec 16, 2015 · 46 comments
Closed

Update Hosting API and TestServer API #525

JunTaoLuo opened this issue Dec 16, 2015 · 46 comments
Assignees
Milestone

Comments

@JunTaoLuo
Copy link
Contributor

Several issues and requests for API changes are being addressed in this PR #521.

This addresses: #518 #504 #357 #298 .

@JunTaoLuo
Copy link
Contributor Author

Follow up items tracked in #530

@guardrex
Copy link
Contributor

Do these changes go into effect with the current DNX -16343? ... or the next one?

[EDIT]

'Microsoft.AspNet.Server.Kestrel' does not contain a 'Program' type suitable for an entry point
'PlatformIssue' does not contain a 'Program' type suitable for an entry point

capture

@Tratcher
Copy link
Member

This change does not affect DNX.

@guardrex
Copy link
Contributor

When do we make the changes? Obviously, I'm confused.

@kevinchalet
Copy link

@guardrex you need to add a Main method in your project and update your web command to use your own entry point:

public static void Main(string[] args) {
    var application = new WebApplicationBuilder()
        .UseConfiguration(WebApplicationConfiguration.GetDefault(args))
        .UseStartup<Startup>()
        .Build();

    application.Run();
}
"web": "Microsoft.AspNet.Server.Kestrel"

->

"web": "PlatformIssue"

@guardrex
Copy link
Contributor

@PinpointTownes Gotcha ... thx ... Adding the Main wasn't clear from what @JunTaoLuo posted. I work exclusively with web apps. Having a Main isn't something I normally care to have. 😄

@davidfowl
Copy link
Member

Yes, this is a change that's there today in the RC1 templates as well (though it's not wired up properly). Your application now controls the entry point.

@guardrex
Copy link
Contributor

I got it ... It works. Thanks! I'm good now.

@kevinchalet
Copy link

Yes, this is a change that's there today in the RC2 templates as well (though it's not wired up properly). Your application now controls the entry point.

Sounds like another regression, IMHO. But I guess it's the price to pay to embrace the new CLI world? 😄

@kevinchalet
Copy link

Oh, and FWIW, I preferred the tiny version of the entry point: WebApplication.Run<Startup>() (or something like that) 😄

@davidfowl
Copy link
Member

Sounds like another regression, IMHO. But I guess it's the price to pay to embrace the new CLI world? 😄

It's actually pretty nice in some ways. The application gets to control all aspects of startup.

Oh, and FWIW, I preferred the tiny version of the entry point: WebApplication.Run() (or something like that) 😄

It's great until you need to customize anything. That's why we did this refactoring. People wanted to add overloads to WebApplication.Run and it was getting out of hand.

@guardrex
Copy link
Contributor

This was my Christmas present from the team 🎄 ... a Main ... just what I always wanted and in my size, too.

If we're going to break with dotnet CLI changes in either VS or on the server, please give us a shoutout warning to "freeze" our runtime and packages so we can ride it out to -rc2-final.

@kevinchalet
Copy link

It's great until you need to customize anything. That's why we did this refactoring. People wanted to add overloads to WebApplication.Run and it was getting out of hand.

WebApplication.Start/Run was more or less the equivalent of Katana's WebApp and I've never heard someone trying to customize it, so I guess it's rather a niche scenario (except maybe for those who want to externalize the startup class in another assembly). The best compromise would be to use WebApplication.Run by default and suggest advanced users to use WebApplicationBuilder when needed.

@davidfowl
Copy link
Member

WebApplication.Start/Run was more or less the equivalent of Katana's WebApp and I've never heard someone trying to customize it, so I guess it's rather a niche scenario (except maybe for those who want to externalize the startup class in another assembly).

We did, that's by we designed the new API this way. We heard complaints about the amount of overloads WebApplication.Start had and I actually agreed. It got out of hand pretty quickly. Not to mention we have more things you can customize in this new world.

The best compromise would be to use WebApplication.Run by default and suggest advanced users to use WebApplicationBuilder when needed.

We met and decided against that because the cliff you fall off when you need to customize anything is too big. We're sticking with this API and we'll make it easy to do the simple things so that it scales from 0 to 100.

@tenor
Copy link

tenor commented Dec 22, 2015

For projects with multiple commands, Do we declare multiple entry points (entry points with names other than Main) or do we need to create a new class for each entry point?

@egorpavlikhin
Copy link

Running IIS Express through Visual Studio changes project.json from

{
  "commands": {
   "web": "{your namespace}"
 }
}

back to Microsoft.AspNet.Server.Kestrel
of course nothing runs after that.

@guardrex
Copy link
Contributor

@JunTaoLuo For Item 4 Startup assembly name ...

Previously, when no startup is specified, we default to using the current assembly when nothing is specified. This has been removed so now an explicit Startup must be specified through WebApplication.Configure(...), WebApplication.UseStartup(...) or through hosting.json.

... it looks like my test app is running without an explicit Startup and not throwing any exception or misbehaving AFAIK. The test code I'm working with can be seen here: dotnet/AspNetCore.Docs#825 (comment).

Your intention is that .UseStartup<Startup>() is absolutely required, correct? If so, I'll add it back.

@Tratcher
Copy link
Member

Your linked code calls Configure which is an alternative for UseStartup.

@guardrex
Copy link
Contributor

@Tratcher .Configure(app => { ... }) ... ahhh ... gotcha. More coffee this morning! Gotta help! Thx

@JaymzZh
Copy link

JaymzZh commented Dec 24, 2015

After this update, I can't run it on VS2015, I got this : Error: Unable to load application or execute command 'ZBlog'. Available commands: web., but I can run it with cmd: dnx web

@davidfowl
Copy link
Member

@jeffiy did you change your application to account for the new APIs?

@JaymzZh
Copy link

JaymzZh commented Dec 24, 2015

@davidfowl Yeah, I did changed.

@davidfowl
Copy link
Member

Put a sample somewhere with repro steps

@JaymzZh
Copy link

JaymzZh commented Dec 24, 2015

@davidfowl You can see the sample here, thanks.

@timmydo
Copy link

timmydo commented Dec 31, 2015

Is there a sample that is set up to run in an azure web app? I can run on my local box (and kestrel defaults to localhost:5000), but on azure I'm not sure of the right way to hook it up to IIS. The generated web.config from dnu publish is different than the ones back I have from the KRE days.

@davidfowl
Copy link
Member

@timmydo Did you update to the latest ASP.NET 5? Are you using visual studio? It'll "do the right thing". Trying to update a project from KRE days will be hard, too much has changed.

@timmydo
Copy link

timmydo commented Dec 31, 2015

@davidfowl Using VS 2015 (but publishing separately from command line) with 1.0.0-rc2-16357 clr x64 and https://www.myget.org/F/aspnetvnext . dnu publish creates a web.config like

<configuration>
  <system.webServer>
    <handlers>
      <add name="httpplatformhandler" path="*" verb="*" modules="httpPlatformHandler" resourceType="Unspecified" />
    </handlers>
    <httpPlatform processPath="..\approot\GearUp.cmd" arguments="" stdoutLogEnabled="true" stdoutLogFile="..\logs\stdout.log" forwardWindowsAuthToken="false" startupTimeLimit="3600"></httpPlatform>
  </system.webServer>
</configuration>

I can run the cmd file by itself and everything works on port 5000. With the move to the httpplatformhandler, I was looking for examples on setting up kestrel to listen under httpplatformhandler (which would need to use HTTP_PLATFORM_PORT, right?) and how the program entry point should look. I got it working on azure with the code below after a bunch of tries, but I didn't see any samples running under httpplatformhandler in the samples @JunTaoLuo posted.

        public static void Main(string[] args)
        {
            Console.WriteLine("Args:\n" + string.Join("\n", args));

            var config = WebApplicationConfiguration.GetDefault(args);

            var application = new WebApplicationBuilder()
                .UseConfiguration(config)
                .UseStartup<Startup>()
                .Build();

            string port = System.Environment.GetEnvironmentVariable("HTTP_PLATFORM_PORT");
            if (string.IsNullOrEmpty(port))
            {
                port = "5000";
            }

            Console.WriteLine("Port: " + port);

            var addresses = application.GetAddresses();
            addresses.Clear();
            addresses.Add("http://localhost:" + port);

            try
            {
                application.Run();
            }
            finally
            {
                Console.WriteLine("Exiting...");
            }
        }

@davidfowl
Copy link
Member

Why aren't you using RC1? RC2 isn't done yet and as @guardrex said, there are changes that are still is flight (as RC2 is still under development). Anyways, you don't need to manually read the port from the environment variable, the default hosting configuration already handles that.

@timmydo
Copy link

timmydo commented Dec 31, 2015

Ah you're right that the default hosting config does that. My bug was that I was doing something like:

using (app.Start())
{
Console.ReadLine();
}

...and azure/IIS probably closed stdin causing my app to exit early.

The way I see it, I'll have to migrate the code when rc2 comes out anyways, so I might as well do it now. I think there was something from rc2 I wanted so I moved early. The github announcements of breaking changes really makes it easier to migrate when names change.

@KallDrexx
Copy link

Quick question.

I am working on a console application that has Asp.Net MVC embedded in it (for health checks and to receive notifications), and I have been struggling to figure out how to share the IoC container between the console portion and Asp.Net portion without using a static/global container property in RC1 ( with WebHostBuilder).

Is it correct to assume that this API change in RC2 will allow this by passing in a specific delegate into WebApplicationBuilder.ConfigureServices(x => PrivateConfigureServicesMethod(X));?

@davidfowl
Copy link
Member

Is it correct to assume that this API change in RC2 will allow this by passing in a specific delegate into WebApplicationBuilder.ConfigureServices(x => PrivateConfigureServicesMethod(X));?

That doesn't let you override the IServiceProvider, it merely lets you add more services that the Startup's ConfigureServices will see. Not sure if that's what you want.

@KallDrexx
Copy link

I did want to override the IServiceProvider, but if that delegate would allow me to pull out the IServiceProvider so I can pass that from my web host management class back down into my console app portion that would work too.

Unless there is a better way to deal with this situation that I am unaware of? I really don't want to maintain two separate IoC containers for the different portions of the application and hope that any changes I make in resolution strategies is consistent between them (especially when it comes to singleton and instance resolutions).

@davidfowl
Copy link
Member

I did want to override the IServiceProvider, but if that delegate would allow me to pull out the IServiceProvider so I can pass that from my web host management class back down into my console app portion that would work too.

That's possible, once you build the application you can access IWebApplication.Services.

@KallDrexx
Copy link

That's possible, once you build the application you can access IWebApplication.Services.

Looking again I guess that's also possible with RC1 via IApplication.Services property. I guess my only apprehension around that is it's forcing me to rely on Asp.Net to be in charge of IoC rather than letting my underlying infrastructure be in charge of IoC and asking Asp.Net to utilize it, which seems backwards since my application is primarily a console application that also utilizes Asp.Net.

@davidfowl
Copy link
Member

I did want to override the IServiceProvider.

So what you're saying is that you do want to do this.

@davidfowl
Copy link
Member

This conversation might go more smoothly if you wrote a code samples showing exactly what you want to do. A sample repository on github showing what that looks like would be even better.

@KallDrexx
Copy link

Sorry for the confusion.

I will work on getting a sample project of what I'm trying to do up on Github after lunch (current prototype is entangled a bit in a proprietary solution so I can't upload it as is).

Essentially my primary goal is to utilize the exact same ioc container instance for both console and asp.net portions of my Console application that embeds Asp.Net. I do not want each portion to have a different ioc container because i want to make sure that anything that resolves anywhere in the application is consistent from where it resolves in any other part of the application (especially critical for singletons and instanced lifetime objects).

My initial thinking was to wrap my ioc logic in an implementation of IServiceProvider and pass that to Asp.Net. This does not seem to be a desireable goal now that I've dug into the code a bit more because WebApplicationBuilder.BuildHostingServices() seems to do a lot of required default registrations that do not get applied if I override the WebApplication.ApplicationServices. The interface doesn't have a good method for adding new service definitions or to list out existing ones,

With that seemingly not being a viable solution the only other solution that's apparent to me is to create code that creates an IServiceCollection for all of my custom resolution streategies, then pass that to Asp.Net via WebHostBuilder.ConfigureServices(), add all of my ServiceDefinition objects to Asp.Net's IServiceCollection, then after Asp.Net is spun up use IApplication.ApplicationServices to retrieve that and return the IServiceProvider back to the main portion of my application so the non-asp.net portions of the app can resolve things consistently.

I'm not a fan of the latter idea because it relies on my primarily console application to rely on Asp.Net for dependency injection.

I am not aware of a cleaner way to really keep dependency injection consistent throughout my app (I could very well be missing something since this is all being done via exploratory testing and not much documentation).

@KallDrexx
Copy link

Sorry for the delay.

An example of my scenario is at https://github.com/KallDrexx/ConsoleAspNetExample/tree/master/src/ConsoleAspNetExample. This is the only solution I can figure out to get dependency injection to work the same in and out of Asp.Net, which is especially useful when singletons are involved. I might be missing something stupid here and I'm open to that criticism.

The problem with this is that my core application is now reliant on getting the ioc container from Asp.Net, and due to the IServiceProvider interface it is immutable from that point on (so I have to figure everything out before starting Asp.Net). This seems like odd coupling since the main purpose of this application has nothing to do with Asp.Net, it's just providing some endpoints for some external communication.

To me, in an ideal world, I could create my ioc container in the Main() method (using this code as an example), pass that to the WebHost.Start() method, and that method just tells Asp.Net to use that ioc container and add what it needs to it.

Hopefully that makes more sense.

@davidfowl
Copy link
Member

You can't add to an already constructed container. What IOC container are you using? Does it have an adapter for asp.net 5? I think you can do what you want but I need some more details filled in

@KallDrexx
Copy link

I don't have a preferred ioc container in mind as the ones I usually use don't have good .net core support yet. I was planning on just using Autofac since it seems to have good .net core support (and is referenced in the Asp.net 5 documentation) but in the end I don't really care what ioc container I use, i just need one that works and makes me not have to worry about dependency injection.

@davidfowl
Copy link
Member

It matters what IOC container you use because most of them don't allow you to modify the container itself after it's been built. The only place you can get at the container registrations is the startup class. You have to use ConfigureServices in the startup class then use that instance in the rest of your application.

public IServiceProvider ConfigureServices(IServiceCollection services)
{
      _containerBuilder.Populate(services);
     _container = _containerBuilder.Build();
     return _container.Resolve<IServiceProvider>();
}

The problem though is that you need to flow out the IOC container you built in ConfigureServices. There's no way around that.

@KallDrexx
Copy link

Ok so I guess I've been jaded by my recent use of Ninject and TinyIOC that's allowed me to add resolutions to an existing container, and forgot that

However, it seems to me that this makes Asp.Net very non-composable. If we say that Asp.net must always be in charge of creating your ioc container (no matter how small a part asp.net may actually be in your infrastructure) what's to say that I don't need another piece of infrastructure that's making the same assumption. Then I have zero possibility of having sane IOC in my project at this point because two totally unrelated (and marginal in my use case) pieces of infrastructure have made global and conflicting assumptions about how my application should be designed.

WebApplicationBuilder actually has everything I need in the BuildHostingServices() method, but it's private so I can't actually call that to stand up my own IServiceProvider instance.

Honestly, I almost wish WebApplicationBuilder.BuildHostingServices() was public static (or somewhere unrelated but accessible) with the configureServices section removed. Then I could do something like:

    public class WebHost : IDisposable
    {
        private IHostingEngine _hostingEngine;
        private IApplication _application;

        public IServiceCollection GetServices()
        {
            var services = WebHostBuilder.GetDefaultServices();
            services.AddMvc();
            return services;
        }

        public void Start(IServiceProvider serviceProvider)
        {
            _hostingEngine = new WebHostBuilder(GetConfig())
                .UseServer("Microsoft.AspNet.Server.Kestrel")
                .UseServiceProvider(serviceProvider)
                .UseStartup<WebHost>()
                .Build();

            _application = _hostingEngine.Start();
        }

        public void Dispose()
        {
            _application?.Dispose();
        }
    }

........

        public static void Main(string[] args)
        {
            var services = GetApplicationServices();

            using (var host = new WebHost())
            {
                var webServices = host.GetServices();
                foreach (var webService in webServices)
                {
                    services.Add(webService);
                }

                var container = CreateIocContainer();
                host.Start(container);
            }
        }

Then (theoretically) the WebHostBuilder.Build() checks if a IServiceProvider was given, if so then it ignores the building of services, gets the resolution it needs and passes it into WebApplication's constructor. I do see that WebApplication adds to the collection but that can also be abstracted away by an IWebApplication..GetRequiredServices() method.

This allows clients to manage the creation of service providers instead of forcing Asp.Net to always be responsible for that.

@davidfowl
Copy link
Member

Something like that might be possible with some big changes (we'd have to shuffle some things around a bit) but we're not making anymore big API changes at this point. Can you file another issue so we can track this work to do something about it post RTM? There's likely a bunch of things that would need to change and be considered to make this work well end to end with other containers.

@KallDrexx
Copy link

Created #550.

Thanks for the responses!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants