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.

Sign up for GitHub

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

Jump to bottom

Use ApplicationStatus health check in ServiceDefaults #7151

Merged
merged 3 commits into from
Jan 27, 2025
Merged

Use ApplicationStatus health check in ServiceDefaults #7151

merged 3 commits into from
Jan 27, 2025

Conversation

matthebrown
Copy link
Contributor

@matthebrown matthebrown commented Jan 19, 2025 •
edited
Loading

Description

Replaces the default "live" health check in the ServiceDefaults templates with the ApplicationStatus health check from the AspNetCore.HealthChecks.ApplicationStatus package.

Fixes #4110

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
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jan 19, 2025
@matthebrown
Copy link
Contributor Author

@mbrown379 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

@dotnet-policy-service agree

@matthebrown matthebrown mentioned this pull request Jan 19, 2025
Closed
@matthebrown
Copy link
Contributor Author

matthebrown commented Jan 19, 2025 •
edited
Loading

Requires AspNetCore.HealthChecks.ApplicationStatus to be added to one of the feeds.

@DamianEdwards
Copy link
Member

You've only added it to Playground. You'll need to update the templates themselves.

@DamianEdwards
Copy link
Member

Requires AspNetCore.HealthChecks.ApplicationStatus to be added to one of the feeds.

I've kicked off this mirroring request and will re-run the pipelines once it completes.

@DamianEdwards
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@matthebrown
Copy link
Contributor Author

Requires AspNetCore.HealthChecks.ApplicationStatus to be added to one of the feeds.

I've kicked off this mirroring request and will re-run the pipelines once it completes.

Thanks, will address your comments and push new changes.

DamianEdwards
DamianEdwards requested changes Jan 22, 2025
View reviewed changes
Copy link
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.

Some things to address in comments.

@dotnet-policy-service dotnet-policy-service bot added the needs-author-action An issue or pull request that requires more info or actions from the author. label Jan 22, 2025
@matthebrown matthebrown force-pushed the AddApplicationStatusHealthCheck branch from fd61917 to 6da0cdf Compare January 22, 2025 17:23
@dotnet-policy-service dotnet-policy-service bot removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Jan 22, 2025
DamianEdwards
DamianEdwards approved these changes Jan 22, 2025
View reviewed changes
Copy link
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.

Looks good. Thanks!

@build-analysis build-analysis bot mentioned this pull request Jan 22, 2025
Open
@DamianEdwards DamianEdwards merged commit 4b5f2a3 into dotnet:main Jan 27, 2025
9 checks passed
@matthebrown matthebrown deleted the AddApplicationStatusHealthCheck branch January 28, 2025 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DamianEdwards DamianEdwards DamianEdwards approved these changes

Assignees
No one assigned
Labels
community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Consider adding ApplicationStatus health check to ServiceDefaults in place of default "live" check
2 participants
@matthebrown @DamianEdwards