Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support specifying host configuration via WebApplication.CreateBuilder #34837

Closed
davidfowl opened this issue Jul 29, 2021 · 15 comments · Fixed by #34972
Closed

Support specifying host configuration via WebApplication.CreateBuilder #34837

davidfowl opened this issue Jul 29, 2021 · 15 comments · Fixed by #34972
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels feature-minimal-hosting Priority:0 Work that we can't release without
Milestone

Comments

@davidfowl
Copy link
Member

davidfowl commented Jul 29, 2021

Background and Motivation

We've decided that it is not possible to mutate the host configuration (environment, application name, content root) after the call to WebApplication.Create/CreateBuilder this simplifies the implementation as we don't need to handle the environment changing arbitrarily as this issue describes. Today the only way to change host configuration is to specify them in a command line args format which isn't very ergonomic. We should add an API that lets the user configure host configuration specifically as part of the Create/CreateBuilder call.

Proposed API

namespace Microsoft.AspNetCore.Builder
{
    public class WebApplication
    {
-       public static WebApplicationBuilder CreateBuilder(string[]? args = null);
+       public static WebApplicationBuilder CreateBuilder();
+       public static WebApplicationBuilder CreateBuilder(string[] args);
+       public static WebApplicationBuilder CreateBuilder(WebApplicationBuilderOptions options);
    }

+   public sealed class WebApplicationBuilderOptions
+   {
+       public string[]? Args { get; init; } 
+       public string? EnvironmentName { get; init; }
+       public string? ApplicationName { get; init; }
+       public string? ContentRootPath { get; init; }
+ }
}

Usage Examples

var config = new WebApplicationOptions 
{
    Args = args,
    EnvironmentName = Environments.Staging,
    HostingStartupAssemblies = "Plugins",
};

var app = WebApplication.Create(options);

app.MapGet("/", (IHostEnvironment env) => env.EnvironmentName);

app.Run();

Alternative Designs

namespace Microsoft.AspNetCore.Builder
{
    public class WebApplication
    {
-       public static WebApplication CreateBuilder(string[]? args = null);
+       public static WebApplicationBuilder CreateBuilder();
+       public static WebApplicationBuilder CreateBuilder(string[] args);
+       public static WebApplicationBuilder CreateBuilder(IConfiguration configuration);
    }
}

Args will win to allow hosts like WebApplicationFactory to override application behavior. This is important for testing.

var config = new ConfigurationManager();
config.AddCommandLine(args);
config[HostDefaults.EnvironmentKey] = Environments.Development;
config[WebHostDefaults.HostingStartupAssembliesKey] = "Plugins";

var app = WebApplication.Create(config);

app.MapGet("/", (IHostEnvironment env) => env.EnvironmentName);

app.Run();

Risks

None.

PS: I removed these options from WebApplicationOptions because they are super advanced and are typically controlled by the environment. We can always add them but it seems like it might be overkill here. Open to suggestions of course.

public string? HostingStartupAssemblies { get; init; }
public string? HostingStartupExcludedAssemblies { get; init; }
public bool? PreventHostingStartup { get; init; }

Also I only changed CreateBuilder, and not Create because this is a more advanced scenario.

@davidfowl davidfowl added api-suggestion Early API idea and discussion, it is NOT ready for implementation api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews feature-minimal-hosting and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jul 29, 2021
@Pilchie Pilchie added the area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Jul 29, 2021
@jkotas
Copy link
Member

jkotas commented Jul 30, 2021

Note that the specifying assemblies as strings in HostingStartupAssemblies is not trimming friendly. There is no good way for the trimmer to tell that it should keep the assemblies in this list around. It should be marked with RequiresUnreferencedCode attribute.

@davidfowl davidfowl self-assigned this Jul 30, 2021
@davidfowl davidfowl added this to the 6.0-rc1 milestone Jul 30, 2021
@davidfowl
Copy link
Member Author

Note that the specifying assemblies as strings in HostingStartupAssemblies is not trimming friendly. There is no good way for the trimmer to tell that it should keep the assemblies in this list around. It should be marked with RequiresUnreferencedCode attribute.

That entire feature isn't trimming friendly, it's our version of startup hooks. We haven't done the work in .NET 6 to make ASP.NET Core trim friendly so these annotations will come en masse as part of .NET 7.

@davidfowl davidfowl added the Priority:0 Work that we can't release without label Jul 31, 2021
@ghost
Copy link

ghost commented Aug 3, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

3 similar comments
@ghost
Copy link

ghost commented Aug 3, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@ghost
Copy link

ghost commented Aug 3, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@ghost
Copy link

ghost commented Aug 3, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@davidfowl
Copy link
Member Author

@mkArtakMSFT @pranavkm is the bot broken? Why does it send this message every time I change the API?

@ghost
Copy link

ghost commented Aug 3, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

1 similar comment
@ghost
Copy link

ghost commented Aug 3, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@davidfowl davidfowl changed the title Support specifying host configuration via WebApplication.Create/CreateBuilder Support specifying host configuration via WebApplication.CreateBuilder Aug 3, 2021
@ghost
Copy link

ghost commented Aug 3, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@ghost
Copy link

ghost commented Aug 3, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

1 similar comment
@ghost
Copy link

ghost commented Aug 3, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@ghost
Copy link

ghost commented Aug 4, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

1 similar comment
@ghost
Copy link

ghost commented Aug 4, 2021

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

  • The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
  • The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
  • Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

@pranavkm
Copy link
Contributor

pranavkm commented Aug 4, 2021

API feedback -> WebApplicationOptions -> WebApplicationBuilderOptions. Discussion about args array being preferred over explicitly set properties. The behavior should be documented as part of the property description.

@pranavkm pranavkm removed the api-ready-for-review API is ready for formal API review - https://github.com/dotnet/apireviews label Aug 4, 2021
@pranavkm pranavkm added the api-approved API was approved in API review, it can be implemented label Aug 4, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 3, 2021
@amcasey amcasey added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jun 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels feature-minimal-hosting Priority:0 Work that we can't release without
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants