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

Configuration coverage #7102

Closed
8 tasks done
guardrex opened this issue Jun 17, 2018 · 15 comments · Fixed by #8050
Closed
8 tasks done

Configuration coverage #7102

guardrex opened this issue Jun 17, 2018 · 15 comments · Fixed by #8050
Assignees
Labels

Comments

@guardrex
Copy link
Collaborator

guardrex commented Jun 17, 2018

Goals

  • Coverage is logically sequenced
  • No (or minimal) duplication of coverage
  • Sufficient detail to explain concepts clearly
  • Samples
    • Illustrate the concepts described in the topics
    • Use RP for 2.x
    • Self-document (pages explain what the sample is demonstrating, a little on how it works, and provide instructions to the dev on how to use it)
  • Appropriate cross-linking/indexing

Most of the coverage in @tillig's post Deep Dive into Microsoft Configuration is present in the docs. We'll improve organization and content quality on this pass. I'll react to @tillig's comment below to ensure the docs are applicable (or understood to be applicable) to any .NET Core app that wants config.

Suggestions

  1. Configuration landing page - Currently focused on the Configuration API (config providers) with some additional configuration subject matter (e.g., environment config).
    • The landing page would be better suited to a general explanation of configuration and links out to content pertaining to configuration.
    • Link out to service configuration (Application startup and Dependency injection). Link out to pipeline configuration (Application startup). Link to Configuration from Application startup.
    • Briefly explain how app configuration works (host and app). Link out to Web Host and Generic Host topics for details.
    • Possibly link out to subject matter configuration sections of other topics (specifically where it pertains to IConfiguration). However in many cases, the content might be passing mentions and not worth linking. If there are some opportunities here, then list and link out to those scenarios.
    • Briefly explain Options. Explain how core scenarios rely upon Options for configuration (e.g., IOptions<IISOptions>). Link out to the Options topic for details.
  2. Current Configuration landing page (Configuration API)
    • Move to a subtopic under the landing page.
    • The several console demo apps remain for 1.x only. For 2.x, supply a new RP app that demonstrates the config providers and basic concepts. As mentioned, move the general configuration guidance in this topic out to the landing page so that this topic can focus on the Configuration API and config providers.
  3. Look for opportunities to clarify how config providers interact with host and app config. This requires looking at how config is described among the Configuration API, Web Host, and Generic Host topics.
  4. Look for opportunities to improve coverage on how to set env vars in IDEs (VS, VSC, VS for Mac, CLI). We have content on this, but is it organized, clear, and described in context?
  5. Leave the Options subtopic as it is. However, touch-ups are probably required to the text. It might be nice to provide a list of IOptions used by the framework with links out to those scenarios.

Questions

  1. Why is SetCompatibilityVersion for ASP.NET Core MVC in the Application startup topic? ... just to surface it? The content seems better suited for migration, what's new, release-notes, and MVC content. This section doesn't have anything (directly) to do with app startup AFAIK. Correct me if I'm wrong. Shouldn't the app startup topic stay focused on app startup? What we could do is leave the extension in the example and have a one-liner that links out to MVC for this.

Prior work

Advance PRs to simplify the workload for this issue:

I resolve Document overriding app settings in Azure Portal #6514 in the PR.

Syntax highlighting missing string interpolation #7207 was moved to the DocFX repo issues. It's out-of-scope for us.

@guardrex
Copy link
Collaborator Author

@tillig
Copy link
Contributor

tillig commented Jun 22, 2018

Consider not focusing on ASP.NET Core and ensure the docs are applicable (or understood to be applicable) to any .NET Core app that wants config. I suspect there are people who see a lot of ASP.NET references, think, "Not applicable, I'm writing for Xamarin" (or whatever), and stop reading.

@guardrex
Copy link
Collaborator Author

Thanks for your feedback @tillig. We'll look at this subject in more detail shortly. I have a few other issues to attend first.

Great job on your post. You beat us (me) to it! We've been crazy busy. 🏃 Our priority before reaching this issue is on-going 2.1 updates (e.g., samples). There's a light at the end of the tunnel tho. 🚂 😅

@Rick-Anderson
Copy link
Contributor

That would be a great statement to say upfront.
For most of our customers using ASP.NET Core MVC and even more so, Razor Pages (RP) - there is nothing they can quickly adopt and use. Once we have something RP can easily copy and use, we can focus on the low level.
So the focus should be on RP samples.

@guardrex
Copy link
Collaborator Author

guardrex commented Jul 30, 2018

@Rick-Anderson @scottaddie @Tratcher I've updated the OP ☝️. This is ready for a look/discussion.

Comments? .... Concerns?

@guardrex guardrex removed the blocked label Jul 31, 2018
@Tratcher
Copy link
Member

Tratcher commented Jul 31, 2018

I agree that configuring a wep app is different from the Configuration APIs, but lumping them together under a Configuration topic seems like a recipe for confusion. Do we need two top level categories "Configuring Web Apps" and "Configuration APIs"? Hmm, not sure that's better...

@guardrex
Copy link
Collaborator Author

lumping them together

I propose to do this in a section of the landing topic. I don't plan to provide details there. I'd link out and down to the specifics.

  • Configuration landing page section
    • Explain host config versus app config (overview only)
    • Host config links out to the Web Host/Generic Host topics
    • App config links down to the subtopic Configuration API (or whatever we end up calling it)

I propose to do what you said, but I recommend that we do it in a section early in the Configuration landing page. We can link out and down from there.

Today, the subject of host vs. app config is MIA early in the docs. We leave it up to the dev to piece together what's going on after they're read the "configuration" topic and the "Web Host/Generic Host" topics. A landing page that calls out the basics (and links the content) should solve this problem.

Additionally, we should consider moving the Host node earlier in the TOC. Host is an HORRIBAD position imo. To me, it falls into an early app build/maintain concern. Why do we have it after ...

  • Static Files Middleware
  • Routing
  • URL Rewriting Middleware
  • Logging
  • Handle errors
  • File providers

@Tratcher
Copy link
Member

It seems like the config APIs warrant there own top level landing page for non-asp.net folks. Referencing that from your other landing page is fine.

@guardrex
Copy link
Collaborator Author

guardrex commented Jul 31, 2018

[EDIT] Didn't work to have host vs app config in the Fundamentals landing page. It's buried there. I'll try including a section in the Configuration topic itself.

  • Fundamentals
    • ... Important app-focused topics ...
    • Configuration (config providers) topic (inc. host vs app config)
      • Options topic
    • Host (as we have it today; moved UP in the TOC)
      • Web Host (has the Web Host config details)
      • Generic Host (has the Generic Host config details)
    • ... Less important app-focused topics ...

@Tratcher
Copy link
Member

Sure, nothing wrong with moving Host up.

@guardrex
Copy link
Collaborator Author

guardrex commented Aug 7, 2018

@Tratcher It appears that switch mappings are incompatible with CreateDefaultBuilder. Adding a call to AddCommandLine with a switch mappings dictionary apparently activates too late during startup.

There's no way to add a switch mapping dictionary to the AddCommandLine call inside CreateDefaultBuilder, so a mapped switch chokes immediately with dotnet run.

It seems the only way to do it is to not use CreateDefaultBuilder and do it the old fashioned way. Is that correct?

No 🎲🎲...

public static IWebHostBuilder CreateWebHostBuilder(string[] args)
{
    var switchMappings = new Dictionary<string, string>
        {
            { "-CommandLineKey1", "CommandLineKey1" },
            { "-CommandLineKey2", "CommandLineKey2" }
        };
    
    var config = new ConfigurationBuilder()
        .AddCommandLine(args, switchMappings)
        .Build();
    
    return WebHost.CreateDefaultBuilder(args)
        .UseConfiguration(config)
        .UseStartup<Startup>();
}

This works ...

public static void Main(string[] args)
{
    var switchMappings = new Dictionary<string, string>
        {
            { "-CommandLineKey1", "CommandLineKey1" },
            { "-CommandLineKey2", "CommandLineKey2" }
        };

    var config = new ConfigurationBuilder()
        .AddCommandLine(args, switchMappings)
        .Build();

    var host = new WebHostBuilder()
        .UseConfiguration(config)
        .UseKestrel()
        .UseStartup<Startup>()
        .Start();

    using (host)
    {
        Console.ReadLine();
    }
}

@Tratcher
Copy link
Member

Tratcher commented Aug 7, 2018

That should work... Are you confusing it by passing in args twice? What if you remove args from CreateDefaultBuilder(args)?

@guardrex
Copy link
Collaborator Author

guardrex commented Aug 8, 2018

@Tratcher The ConfigurationBinder doesn't seem to be able to bind recurring element contents (section2 > items below). It chokes saying that the items create duplicate keys in the configuration (i.e., it doesn't use the id value when creating the keys).

<top_level>
  <section0>
    <item0>section0-item0-value</item0>
    <item1>section0-item1-value</item1>
  </section0>
  <section1>
    <items>
      <item id="item0">section1-items-item0-value</item>
      <item id="item1">section1-items-item1-value</item>
      <item id="item2">section1-items-item2-value</item>
    </items>
  </section1>
</top_level>

Is there a way, or is it just not capable of binding this scenario?

@tillig
Copy link
Contributor

tillig commented Aug 8, 2018

@guardrex Use name not id. name has special meaning in XML config. Example on my blog - look at the "ordinal collections" section.

@guardrex
Copy link
Collaborator Author

guardrex commented Aug 8, 2018

Thanks @tillig!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants