Skip to content

Conversation

eerhardt
Copy link
Member

Description

Originally the parent-child lookup contained the "root" parent and all the descendants in a flat list. This doesn't show up well in the dashboard because grandchildren are parented directly under their root.

Fix this by making the parent-child lookup only contain direct children, and recursively parent the descendants on their direct parent.

Fix #7580

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
  • Did you add public API?
    • No
  • Does the change make any security assumptions or guarantees?
    • No
  • Does the change require an update in our Aspire docs?
    • No

Originally the parent-child lookup contained the "root" parent and all the descendants in a flat list. This doesn't show up well in the dashboard because grandchildren are parented directly under their root.

Fix this by making the parent-child lookup only contain direct children, and recursively parent the descendants on their direct parent.

Fix dotnet#7580
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

Copy link
Member

@JamesNK JamesNK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested in TestShop and cosmos E2E and they both work:

image


// the parent name needs to be an instance name, not the resource name.
// parent the children of the child under the first resource instance.
await SetChildResourceAsync(child, child.GetResolvedResourceNames()[0], state, startTimeStamp, stopTimeStamp)
Copy link
Member

@JamesNK JamesNK Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't going to work well with replicas. But I don't think that's new.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you expect it to work with replicas? Which replica should the child go under?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it can work today. Lets see if it comes up before worrying about it.

@JamesNK
Copy link
Member

JamesNK commented Feb 14, 2025

It would be nice if there were some unit tests of RelationshipEvaluator by itself.

@davidfowl
Copy link
Member

Backport plz

@eerhardt eerhardt enabled auto-merge (squash) February 14, 2025 15:33
@eerhardt
Copy link
Member Author

/backport to release/9.1

Copy link
Contributor

Started backporting to release/9.1: https://github.com/dotnet/aspire/actions/runs/13332369112

@eerhardt eerhardt merged commit 9b06baf into dotnet:main Feb 14, 2025
70 checks passed
@eerhardt eerhardt deleted the FixNestedParents branch February 14, 2025 16:11
@github-actions github-actions bot added the area-integrations Issues pertaining to Aspire Integrations packages label Mar 10, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Apr 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-integrations Issues pertaining to Aspire Integrations packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dashboard only allows for one level of nested children
3 participants