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

Expose WebHostBuilderContext in UseKestrel #2177

Merged
merged 1 commit into from
Nov 22, 2017
Merged

Expose WebHostBuilderContext in UseKestrel #2177

merged 1 commit into from
Nov 22, 2017

Conversation

Tratcher
Copy link
Member

@Tratcher Tratcher commented Nov 21, 2017

#1334 @halter73 I've poached this because it will make the configuration work easier.

@Tratcher Tratcher added this to the 2.1.0-preview1 milestone Nov 21, 2017
@Tratcher Tratcher self-assigned this Nov 21, 2017
@Tratcher Tratcher changed the title Expose WebHostBuilderContext in UseKestrel #1334 Expose WebHostBuilderContext in UseKestrel Nov 21, 2017
/// </returns>
public static IWebHostBuilder UseKestrel(this IWebHostBuilder hostBuilder, Action<WebHostBuilderContext, KestrelServerOptions> configureOptions)
{
if (configureOptions == null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this null check be added to the extension above as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

services.Configure does that check.

Copy link
Contributor

@JunTaoLuo JunTaoLuo left a comment

Choose a reason for hiding this comment

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

I don't think the sample app builds

{
.UseKestrel((context, options) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look right

@davidfowl
Copy link
Member

I wonder if this would be better to put on the KestrelServerOptions object itself.

@Tratcher
Copy link
Member Author

                .UseKestrel(options =>
                  {
                    var basePort = options.HostContext.Configuration.GetValue<int?>("BASE_PORT") ?? 5000;
                    if (options.HostContext.HostingEnvironment.IsDevelopment()) { ... }

shrug

It may mess up services.Configure<KestrelServerOptions>(options => ...), or you'd have to capture it like you capture the services today.

@davidfowl
Copy link
Member

It may mess up services.Configure(options => ...), or you'd have to capture it like you capture the services today.

Is the WebHostBuilderContext in the container?

@Tratcher
Copy link
Member Author

No

@davidfowl
Copy link
Member

Then this is fine

@Tratcher Tratcher merged commit 8e1da5d into dev Nov 22, 2017
@Tratcher Tratcher deleted the tratcher/config branch November 22, 2017 18:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants