fix: soft-delete instance from DB when instance manager returns 404#183
fix: soft-delete instance from DB when instance manager returns 404#183PierreLeGuen merged 2 commits intomainfrom
Conversation
Instance may have been removed through another channel (e.g. direct compose-api call). Previously a 404 from the instance manager blocked the DB soft-delete, leaving the user stuck with an undeletable instance.
Summary of ChangesHello @PierreLeGuen, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the instance deletion workflow by introducing more robust error handling for external API calls. It specifically addresses a scenario where an instance might have already been removed from the instance manager, ensuring that the local database record can still be soft-deleted without error. This enhancement improves the reliability and user experience of instance management by preventing orphaned database entries. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Code ReviewChange summary: When compose-api returns 404 on instance deletion, soft-delete from DB instead of surfacing an error to the user. No critical issues found.
✅ Approved |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly handles the case where an instance is already deleted from the instance manager (returning a 404) by proceeding with the soft-delete in the database. This prevents instances from getting stuck in an undeletable state. The change is logically sound. The suggestion to improve the code structure for better readability and future maintenance has been retained.
| if status == reqwest::StatusCode::NOT_FOUND { | ||
| tracing::warn!( | ||
| "Instance not found on instance manager (already removed?), proceeding with DB soft-delete: instance_id={}", | ||
| instance_id | ||
| ); | ||
| } else { | ||
| return Err(anyhow!( | ||
| "Agent API delete failed with status {}: instance_id={}", | ||
| status, | ||
| instance_id | ||
| )); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
For better readability and to make it easier to handle other status codes in the future, you could use a match statement here to handle the non-success status codes.
| if status == reqwest::StatusCode::NOT_FOUND { | |
| tracing::warn!( | |
| "Instance not found on instance manager (already removed?), proceeding with DB soft-delete: instance_id={}", | |
| instance_id | |
| ); | |
| } else { | |
| return Err(anyhow!( | |
| "Agent API delete failed with status {}: instance_id={}", | |
| status, | |
| instance_id | |
| )); | |
| } | |
| } | |
| if !status.is_success() { | |
| match status { | |
| reqwest::StatusCode::NOT_FOUND => { | |
| tracing::warn!( | |
| "Instance not found on instance manager (already removed?), proceeding with DB soft-delete: instance_id={}", | |
| instance_id | |
| ); | |
| } | |
| _ => { | |
| return Err(anyhow!( | |
| "Agent API delete failed with status {}: instance_id={}", | |
| status, | |
| instance_id | |
| )); | |
| } | |
| } | |
| } |
2 participants
Summary
Test plan