Azure / azure-functions-dotnet-worker

Azure Functions out-of-process .NET language worker
MIT License
429 stars 184 forks source link

Update samples and docs to add Host.CreateDefaultBuilder() #1025

Open kshyju opened 2 years ago

kshyju commented 2 years ago

With the recent addition of app insights logging integration on dotnet worker, we should consider exploring the idea of adding an appsettings.json file as a default configuration source to isolated function apps. For customers who are coming from aspnet core webapi background, this experience makes it easier for them.

SeanFeldman commented 2 years ago

Should we have environment specific files (appsettings.{Environment}.json ) ?

I'd love to

  1. Have appsettings.json as the source of truth for the configurations and settings that do not change between team members/environments.
  2. Ditch local.appsetting.config in favour of the standardized appsettings.{Environment}.json for consistency.
brettsam commented 1 year ago

@jviau jsust mentioned this to me the other day. He suggested that we start using CreateDefaultBuilder() with all of our samples to show folks how to wire up default .NET stuff. That way we stay unopinionated about configuration sources, etc, and people learn where that stuff is configured.

Here's everything that this sets up: https://learn.microsoft.com/en-us/dotnet/api/microsoft.extensions.hosting.host.createdefaultbuilder

So samples start looking like...

var host = Host
    .CreateDefaultBuilder()        
    .ConfigureFunctionsWorkerDefaults()
    .Build();
brettsam commented 1 year ago

Should we have environment specific files (appsettings.{Environment}.json ) ?

I'd love to

  1. Have appsettings.json as the source of truth for the configurations and settings that do not change between team members/environments.
  2. Ditch local.appsetting.config in favour of the standardized appsettings.{Environment}.json for consistency.

@SeanFeldman -- that local.appsetting.config file is read by Functions Core Tools and is the only way we have today to inject env vars that the host may need (FeatureFlags, host app settings, etc). It's meant to be language-agnostic, which I'm guessing is why it doesn't follow standard .NET naming.

That being said, I do agree it's confusing I'll spin this comment off into another issue though as I think that allowing us to pass values from appsettings.json into the Core Tools while debugging would require some updates to our tooling and likely to Core Tools.

SeanFeldman commented 1 year ago

@brettsam a focused issue would be a good move.

I think that allowing us to pass values from appsettings.json into the Core Tools while debugging would require some updates to our tooling and likely to Core Tools.

Yes, that will add to your (and potentially other) team's work but imagine a first-class user experience when the well-established configuration approach is consistent between various project types, including Functions. Not to mention that having some configurations in local.appsetting.config and the rest in appsettings.{x}.json breaks that paradigm and will keep being a source of frustration. Unification would remove the annoyance, simplify the functions configuration story, be consistent with the rest of .NET projects, simplify your samples, and reduce the time your team has to invest in explaining why/what/how each time. I think the last one alone is worth all the hassle! 😃

jviau commented 1 year ago

Unification of settings is not straight forward for dotnet isolated. We have two processes here, host and worker. We must always keep some form of separation between settings. Users can have settings that apply to host only, worker only, or both. How do we communicate what applies to what?

breaks that paradigm and will keep being a source of frustration

I don't believe it breaks the paradigm. Our two-process model means the paradigm does not apply to us. At least the single settings file. We will need at least 3 different setting file naming schemes: one is host only, one is worker only. and one is for both. Considering users are already familiar with appsettings.json, we probably should not have that apply to host as that is a potential regression (new host version rolls out and suddenly appsettings.json starts applying to host, what if a value in there adversely impacts the host?)

So, how can we improve this experience, but still accommodate our two-process model and what it means for reading settings files.