Make error handling for already deleted runtime more precise

[kgang]

Encountered this while cleaning up the staging environment. Going to test it out shortly after on staging to confirm that the log is going through to AWS as expected.

---

Important

Enhance error handling in delete_runtime() by logging ClientError and warning if AWS resource might already be deleted.

• Error Handling:
  • In delete_runtime() in tasks.py, wrap ecs_client.delete_service and waiter.wait in a try-except block.
  • Log ClientError and issue a warning if AWS resource not found, suggesting it might be already deleted.

^{This description was created by for 825f879. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 19e3964 in 2 minutes and 23 seconds. Click for details.

• Reviewed 42 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. apps/api/src/tasks.py:360

• Draft comment:
  Ensure that botocore.errorfactory.ServiceNotFoundException is the correct exception to handle an already deleted service. In some cases the AWS client may instead raise a ClientError with an error code indicating the service is missing.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a speculative comment asking to verify exception handling. It's not definitively showing that there's a problem, just suggesting there might be one. The comment starts with "Ensure that..." which is a red flag. We don't have evidence that ClientError is actually thrown in this case. The comment could be correct - AWS APIs sometimes do throw different exceptions for the same condition. Maybe there's documentation showing both exceptions can occur. Even if both exceptions can occur, this is a speculative comment asking for verification rather than definitively pointing out a bug. The current code handles the service-not-found case gracefully. Delete this comment. It's speculative and starts with "Ensure that...", asking the author to verify something rather than pointing out a clear issue.

2. apps/api/src/tasks.py:373

• Draft comment:
  Consider logging additional context (e.g. service name and cluster) in the warning message to aid in debugging.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_1SqNFRw302Y2f2Y6

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 825f879 in 1 minute and 6 seconds. Click for details.

• Reviewed 14 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. apps/api/src/tasks.py:374

• Draft comment:
  Catching the broad ClientError here may mask issues other than a non-existent ECS service. Consider inspecting the error code (e.g., error.response['Error']['Code']) to ensure it’s the specific 'ServiceNotFound' scenario. Also, logging the error and then a generic warning can be combined or clarified for better diagnostics.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 30% vs. threshold = 50% The comment makes a valid point - catching ClientError is broader than the previous ServiceNotFoundException. However, in a deletion function, broad error handling may be intentional since we want to continue with cleanup even if various AWS resources are missing. The logging suggestion is minor and stylistic. The function has a final catch-all exception handler anyway. I may be underestimating the risks of catching broad AWS exceptions. There could be other ClientErrors that should cause the deletion to fail. While true, this is a deletion function that already has a catch-all handler. The goal is to clean up as many resources as possible even if some fail. The comment raises valid points but the current error handling approach is reasonable for a cleanup function that should be resilient to missing resources.

Workflow ID: wflow_sc0G70nJMbpOp2WX

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}