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

Generic Host Builder and HostingEnvironment.IsDevelopment #4150

Closed
Eilon opened this Issue Jul 10, 2018 · 20 comments

Comments

Projects
None yet
7 participants
@Eilon
Copy link
Member

Eilon commented Jul 10, 2018

From @dazinator on July 5, 2018 17:7

I am using the new generic host in 2.1.0 as documented here: https://docs.microsoft.com/en-us/aspnet/core/fundamentals/host/generic-host?view=aspnetcore-2.1

When the hosting environment is development, I'd like to add user secrets:

  .ConfigureAppConfiguration((hostContext, configApp) =>
                   {
                       configApp.AddJsonFile("appsettings.json", optional: true);                   
                       if (hostContext.HostingEnvironment.IsDevelopment())
                       {
                           configApp.AddUserSecrets<Program>();
                       }
                       configApp.AddEnvironmentVariables(prefix: "FOO_");
                       configApp.AddCommandLine(args);
                   })

However IsDevelopment always returns false.
Yet I have set the ASPNETCORE_ENVIRONMENT enironment variable. Here is my launch settings:

{
  "profiles": {
    "Foo": {
      "commandName": "Project",
      "environmentVariables": {
        "ASPNETCORE_ENVIRONMENT": "Development"
      }
    }
  }
}

It would make sense if ASPNETCORE_ENVIRONMENT was no longer used to control this for a generic host as we are not in asp.net core land - but in that case is there now a different environment variable name for this, or do we set the Environment name ourselves in code?

Copied from original issue: #3298

@Eilon

This comment has been minimized.

Copy link
Member Author

Eilon commented Jul 10, 2018

From @dazinator on July 5, 2018 17:11

This seems like a sensible workaround, but it would be better if this behaviour was automatic to be consistent with asp.net core scenarios?


.ConfigureAppConfiguration((hostContext, configApp) =>
                   {
                       hostContext.HostingEnvironment.EnvironmentName = System.Environment.GetEnvironmentVariable("NETCORE_ENVIRONMENT");

@muratg muratg added this to the 3.0.0 milestone Aug 28, 2018

@muratg

This comment has been minimized.

Copy link
Member

muratg commented Aug 28, 2018

Tentatively putting this in 3.0.

pinging @davidfowl @Tratcher for their thoughts.

@dazinator

This comment has been minimized.

Copy link

dazinator commented Aug 29, 2018

@Tratcher
If its not expected to work the same, having the IsDevelopment() API exposed still is a little misleading. Especially for those with asp.net core experience, they will see this API (like I did) and expect to be able to use it in the same way.

                       if (hostContext.HostingEnvironment.IsDevelopment())
                       {
                          
                       }

If this API was not present, I wouldn't have raised an issue. On Dev machines, this always returned true by default until now so that might catch people out.

@davidfowl

This comment has been minimized.

Copy link
Member

davidfowl commented Aug 29, 2018

To be practical here we might just have to recognize ASPNETCORE_ENVIRONMENT in the generic host.

@Tratcher

This comment has been minimized.

Copy link
Member

Tratcher commented Aug 29, 2018

I meant that no environment variables are recognized by default, that's opt-in. IsDevelopment doesn't check ASPNETCORE_ENVIRONMENT, it checks config["environment"], and the environment variable config provider deals with prefixes like ASPNETCORE_. That's all under your control setting up the HostBuilder.

We'll see what defaults we decide to bring back, or if we mirror the CreateDefaultBuilder pattern to get something bootstrapped for you.

@dazinator

This comment has been minimized.

Copy link

dazinator commented Aug 29, 2018

@Tratcher got ya. Ok thanks.

@rbugginsvia

This comment has been minimized.

Copy link

rbugginsvia commented Nov 19, 2018

I've just been having issues with this and i noticed that the environment is not set from ConfigureAppConfiguration but from ConfigureHostConfiguration on the HostBuilder

@natemcmaster natemcmaster transferred this issue from aspnet/Hosting Nov 20, 2018

@pksorensen

This comment has been minimized.

Copy link

pksorensen commented Dec 9, 2018

From a user that dont already read the docs i did use some time here to find out that the enviromentname is defaulted to production ( also causing some pain to find out it altered production environment).

I think it would have been a good idea to add environment variables as default to hostconfiguration :)
and use a fallback on aspnetcore_environment.

I just added to my solution

              .ConfigureHostConfiguration((configurationBuilder)=>{
                  configurationBuilder
                    .AddInMemoryCollection(new[] { new KeyValuePair<string, string>(HostDefaults.EnvironmentKey, System.Environment.GetEnvironmentVariable("ASPNETCORE_ENVIRONMENT")) })
                    .AddEnvironmentVariables();
              })

@Eilon Eilon added the area-hosting label Jan 7, 2019

@muratg

This comment has been minimized.

Copy link
Member

muratg commented Jan 9, 2019

@davidfowl @Tratcher Are we doing something here for 3.0?

@Tratcher

This comment has been minimized.

Copy link
Member

Tratcher commented Jan 10, 2019

Yes this needs design.

@davidfowl davidfowl added this to the 3.0.0-preview3 milestone Jan 18, 2019

@Tratcher

This comment has been minimized.

Copy link
Member

Tratcher commented Jan 22, 2019

There's a PR updating the docs around this:
aspnet/Docs#10531

In 3.0 we've added Host.CreateDefaultBuilder that adds unprefixed environment variable support, so generic host apps are expected to use ENVIRONMENT=Development.

The 3.0 web host integration for generic host adds ASPNETCORE_ prefixed environment variables so web apps can use either the generic ENVIRONMENT or the web specific ASPNETCORE_ENVIRONMENT.

The only remaining question here is if Host.CreateDefaultBuilder should use a prefix when adding environment variables? @glennc?

@Tratcher

This comment has been minimized.

Copy link
Member

Tratcher commented Feb 1, 2019

Proposal: "DOTNET_"

@davidfowl

This comment has been minimized.

Copy link
Member

davidfowl commented Feb 2, 2019

I think we should support both DOTNET_ and ASPNETCORE_

@muratg

This comment has been minimized.

Copy link
Member

muratg commented Feb 2, 2019

@davidfowl Which one overrides the other, in case both are specified?

@davidfowl

This comment has been minimized.

Copy link
Member

davidfowl commented Feb 2, 2019

In that odd case DOTNET wins.

@Tratcher

This comment has been minimized.

Copy link
Member

Tratcher commented Feb 2, 2019

Adding the web hosted service already adds ASPNETCORE_ and overwrites the prior one.

@davidfowl

This comment has been minimized.

Copy link
Member

davidfowl commented Feb 2, 2019

Ah yes maybe that's enough then? If you get the IHostingEnvironment from hosting after calling ConfigureWebHost does it report IsDevelopment() true then?

@Tratcher

This comment has been minimized.

Copy link
Member

Tratcher commented Feb 2, 2019

Yes

@davidfowl

This comment has been minimized.

Copy link
Member

davidfowl commented Feb 2, 2019

Lets write a test for that case to make sure it's covered. The original plan sounds fine in that case. It does mean in the new worker template the DOTNET_ENVIRONMENT flag should be set instead of ASP.NET Core environment. We should also determine if we want to chose the new web templates to use the DOTNET_ENVIRONMENT in launch settings.

cc @glennc

@Tratcher Tratcher closed this Feb 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment