Skip to content

Add WithHttpHealthCheck remarks and Debug.Assert for external services (#10524)#17281

Merged
DamianEdwards merged 2 commits into
microsoft:mainfrom
alirezafzali:fix/external-service-health-tests
May 20, 2026
Merged

Add WithHttpHealthCheck remarks and Debug.Assert for external services (#10524)#17281
DamianEdwards merged 2 commits into
microsoft:mainfrom
alirezafzali:fix/external-service-health-tests

Conversation

@alirezafzali
Copy link
Copy Markdown
Contributor

Description

Follow-up on review comments from #10493 for ExternalServiceResource.WithHttpHealthCheck (#10524).

Changes:

  • Expand XML <remarks> on WithHttpHealthCheck to document static URLs (AddUrlGroup at configuration time) vs. parameter-based URLs (ParameterUriHealthCheck with GetValueAsync on each probe).
  • Add a Debug.Assert that either Uri or UrlParameter is set, consistent with AddExternalServiceImpl.

Motivation: The async parameter health-check behavior was added in #10493; this PR completes the remaining documentation and debug-assert feedback from that review. No behavioral changes.

Dependencies: None.

Fixes #10524

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
    • No

Copilot AI review requested due to automatic review settings May 19, 2026 22:44
Copy link
Copy Markdown
Contributor

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.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds clarity and a guardrail around how WithHttpHealthCheck behaves depending on whether an external service uses a static URI vs. a parameter-resolved URI.

Changes:

  • Expanded XML docs to describe health check registration behavior for static URIs vs. URL parameters.
  • Added a Debug.Assert to enforce that either Uri or UrlParameter is present before registering the health check.

Comment thread src/Aspire.Hosting/ExternalServiceBuilderExtensions.cs
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17281

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17281"

Copy link
Copy Markdown
Member

@DamianEdwards DamianEdwards left a comment

Choose a reason for hiding this comment

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

Thanks!

@DamianEdwards DamianEdwards enabled auto-merge (squash) May 20, 2026 00:39
@DamianEdwards DamianEdwards merged commit 2d57e64 into microsoft:main May 20, 2026
600 of 603 checks passed
@github-actions github-actions Bot added this to the 13.4 milestone May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Follow up on comments from PR 10493

3 participants