Autowire adapter services for incremental migration with aspire #597
Conversation
|
@CZEMacLeod I'd appreciate any thoughts you have here as well. I know you have some similar APIs in one of your projects, but the goal here would be to plumb things through so they can just work and requires a bit more deeper plumbing to allow the services to be autowired together. |
src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/SystemWebAdapterExtensions.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.SystemWebAdapters.Aspire/IncrementalMigrationResourceExtensions.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.AspNetCore.SystemWebAdapters.Aspire/AspireConstants.Shared.cs
Outdated
Show resolved
Hide resolved
| <ImplicitUsings>enable</ImplicitUsings> | ||
| <Nullable>enable</Nullable> | ||
| </PropertyGroup> | ||
|
|
There was a problem hiding this comment.
Sorry if I'm missing something dumb, but can't these things be lumped together with the IncrementalMigration project as well?
There was a problem hiding this comment.
A couple of reasons why I added a new package:
- It seems that the opinionated Aspire stuff is usually in a separate package, so I followed that pattern
- We don't currently directly reference Yarp in the packages, so this package brings that together
- It also simplifies that for both framework and core, we can now just say install the "Aspire.Microsoft.AspNetCore.SystemWebAdapters" package
I'm not tied to this setup and would be fine combining them if these reasons aren't good ones to keep it separate.
| <PackageVersion Include="System.Runtime.Caching" Version="8.0.0" /> | ||
| <PackageVersion Include="System.Security.Principal.Windows" Version="5.0.0" /> | ||
| <PackageVersion Include="System.Text.Json" Version="8.0.5" /> | ||
| <PackageVersion Include="Yarp.ReverseProxy" Version="2.3.0" /> |
There was a problem hiding this comment.
I know you are not using that here yet, but as FYI Aspire also has a custom integration for Yarp.
There was a problem hiding this comment.
Yeah, I took a look at it, but it seems to require a setup with a config file which isn't how we've been using it since there's a single fallback. Maybe I missed something, but it seems that in its current state it's not a great fit for how customers set up incremental migration setups.
| { | ||
| // All health checks must pass for app to be considered ready to accept traffic after starting | ||
| app.MapHealthChecks("/health"); | ||
| app.MapHealthChecks("/health").ShortCircuit(); |
There was a problem hiding this comment.
Might be a dumb question but won't this be already part of the above call to configure http clients where we add the standard resilience handler?
3 participants
This change enables auto-configuration of the adapter services based off of the configuration set in aspire. This will ensure that things are wired up correctly for each and only has to be declared in a single space.
This is currently a proof of concept to see how much we can simplify the process of incremental migration with Aspire.