Honor PreferHostingUrls #1639
Honor PreferHostingUrls #1639
Conversation
|
nit: English spelling of Honour 😉 |
| @@ -131,8 +131,15 @@ public void Start<TContext>(IHttpApplication<TContext> application) | |||
|
|
|||
| return; | |||
| } | |||
| else if (!hasListenOptions) | |||
| else if (!hasListenOptions || (_serverAddresses.PreferHostingUrls && hasServerAddresses)) | |||
There was a problem hiding this comment.
Not a big fan of how these if else are structured since it's somewhat convoluted. As far as I can tell, it's to reduce code duplication for translating server addresses to listen options and binding to the listen options.
I have an alternative approach added to the branch at https://github.com/aspnet/KestrelHttpServer/compare/johluo/use-hosting-urls?diff=split&expand=1&name=johluo%2Fuse-hosting-urls but there's a bit of moving things around so I'm not sure if it's worth the effort.
There was a problem hiding this comment.
I like the alternative approach; it's easier to follow the logic in that version.
| { | ||
| services.AddSingleton<ILoggerFactory>(new KestrelTestLoggerFactory(testLogger)); | ||
| }) | ||
| .UseLoggerFactory(_ => new KestrelTestLoggerFactory(testLogger)) |
There was a problem hiding this comment.
Why not use the overload that takes an instance?
There was a problem hiding this comment.
We were thinking of obsoleting that overload. aspnet/Hosting#1007. We want to use the overloads that also take in the WebHostBuilderContext.
|
closing in favour of #1655 |
Addresses #1575.
We want to override the endpoints configured in the listen options when PreferHostingUrls is set to true with the addresses in the IServerAddressesFeature if it's not empty.
Will squash with a meaningful commit message once ready to merge.