Skip to content

Conversation

karolz-ms
Copy link
Member

Description

Applies retry logic to resource stopping operation

Fixes # (issue)

Fixes #8481

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?

I ran all Aspire.Hosting and Aspire.Hosting.Testing tests locally and only 2 quarantined tests failed (one passed on re-run). The changes to TestKubernetesService are required to make tests pass.

@karolz-ms karolz-ms requested a review from mitchdenny as a code owner April 4, 2025 01:41
@github-actions github-actions bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Apr 4, 2025
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 2 out of 2 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

tests/Aspire.Hosting.Tests/Dcp/TestKubernetesService.cs:177

  • Consider safely casting patch.Content (using a safe cast or adding a null-check) to avoid potential runtime exceptions if the content is not of type Json.Patch.JsonPatch.
Json.Patch.JsonPatch jsonPatch = (Json.Patch.JsonPatch)patch.Content;

@mitchdenny
Copy link
Member

Backport too please!

@davidfowl davidfowl merged commit 3f9fa6b into main Apr 4, 2025
175 checks passed
@davidfowl davidfowl deleted the dev/karolz/aspire8481 branch April 4, 2025 04:39
@davidfowl
Copy link
Member

/backport to release/9.2

Copy link
Contributor

github-actions bot commented Apr 4, 2025

Started backporting to release/9.2: https://github.com/dotnet/aspire/actions/runs/14258144458

Copy link
Contributor

github-actions bot commented Apr 4, 2025

@davidfowl backporting to "release/9.2" failed, the patch most likely resulted in conflicts:

$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Patch format detection failed.
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@davidfowl
Copy link
Member

@karolz-ms all yours 😄

@karolz-ms
Copy link
Member Author

Honestly IMO this is not super critical to be backported to 9.2 but I will try

Copy link
Contributor

github-actions bot commented Apr 4, 2025

Started backporting to release/9.2: https://github.com/dotnet/aspire/actions/runs/14269439222

@github-actions github-actions bot locked and limited conversation to collaborators May 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Projects
None yet
Development

Successfully merging this pull request may close these issues.

change resource stopping to test and retry
4 participants