-
Notifications
You must be signed in to change notification settings - Fork 648
Ensure resource non-endpoint URLs are active when initialized #9696
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR ensures that non-endpoint resource URLs are marked active immediately when resources are initialized, while endpoint URLs remain inactive until explicitly activated.
- ApplicationOrchestrator now sets
IsInactive
based on whether a URL has an associated endpoint. - Tests in WithUrlsTests.cs have been updated and extended to verify initial active/inactive states for both project and custom resources.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
tests/Aspire.Hosting.Tests/WithUrlsTests.cs | Renamed and extended test, added CustomResource stub, split URL snapshots, updated assertions for active/inactive URLs |
src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs | Changed initial UrlSnapshot.IsInactive logic to url.Endpoint is not null ; moved and adjusted related comment |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@@ -396,50 +397,164 @@ public async Task MultipleUrlsForSingleEndpointAreIncludedInUrlSnapshot() | |||
} | |||
|
|||
[Fact] | |||
public async Task NonEndpointUrlsAreInactiveUntilResourceRunning() | |||
public async Task UrlsAreInExpectedStateForResourcesGivenTheirLifecycle() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was hard to figure out what this test is testing since it looks like you are simulating what should happen, and then asserting that the simulation is correct vs internal behavior (might just be that there is a lot going on in this test).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this test is verifying that the state of the URLs for project resources and custom resources is as expected as those resources move through their lifecycles.
* Ensure resource non-endpoint URLs are active when initialized Fixes #9516 * WIP * Split static URLs upfront * Remove commented out line * Refactoring * Test tweak * Tweak test * Update src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update WithUrlsTests.cs * Fix race in test * Add comment --------- Co-authored-by: Mitch Denny <midenn@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Description
Changes resource URL processing so that all resources get their non-endpoint URLs initialized earlier than today. This is done by changing
WithUrl<T>(this IResourceBuilder<T> builder, string url, string? displayText = null)
to add theResourceUrlAnnotation
directly to the resource, rather than register aResourceUrlCallbackAnnotation
to be processed later. InApplicationOrchestrator
's handler for the DCPOnResourcesPrepared
event, resource's initial state is published, including any static URLs. URL callbacks (viaResourceUrlCallbackAnnotation
) are now processed in a handler for theResourceEndpointsAllocatedEvent
handler at which point all endpoints are expected to be allocated for the resource. The endpoint URLs are published initially in an inactive state. Endpoint URLs must be published as active by some other actor in the future (for DCP-controlled resources this already happens, for custom resources it's up to the resource custom logic to do this).New lifecycle is:
ApplicationOrchestrator
handles DCPOnResourcesPreparedContext
event:InitializeResourceEvent
is published to indicate to the resource it should initialize itself, e.g. start its lifecycleApplicationOrchestrator
handles theResourceEndpointsAllocatedEvent
for each resource:DcpExecutor
for executables, projects, and containers, and by the resource itself for custom resources)IResourceWithoutLifetime
andIValueProvider
are processed (their values are published)Fixes #9516
Checklist