Enhance efficiency for checking VM status#2128
Enhance efficiency for checking VM status#2128cb-github-robot merged 1 commit intocloud-barista:mainfrom
Conversation
Signed-off-by: Seokho Son <shsongist@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR enhances efficiency for VM status checking by preventing unnecessary API calls when a VM is already in its target status, along with reducing log noise by commenting out a warning message.
- Adds status check before making CB-Spider API calls to skip operations when VM is already in target status
- Comments out warning log for missing labels to reduce log verbosity
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/core/infra/control.go | Adds efficiency check to skip CB-Spider API calls when VM is already in target status |
| src/core/common/label/label.go | Comments out warning log message for missing labels |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if currentStatusBeforeUpdating == temp.TargetStatus { | ||
| log.Debug().Msgf("[ControlVmAsync] VM [%s] is already in target status [%s], skipping CB-Spider call", vmId, temp.TargetStatus) | ||
| callResult.Status = temp.Status |
There was a problem hiding this comment.
There's an inconsistency in status assignment. The condition checks if 'currentStatusBeforeUpdating == temp.TargetStatus', but then assigns 'callResult.Status = temp.Status'. This should be 'callResult.Status = currentStatusBeforeUpdating' to maintain consistency with the status that was actually checked.
| if currentStatusBeforeUpdating == temp.TargetStatus { | |
| log.Debug().Msgf("[ControlVmAsync] VM [%s] is already in target status [%s], skipping CB-Spider call", vmId, temp.TargetStatus) | |
| callResult.Status = temp.Status | |
| callResult.Status = currentStatusBeforeUpdating |
| } | ||
| if !exists { | ||
| log.Warn().Msgf("No label found for '%s'", uid) | ||
| // log.Warn().Msgf("No label found for '%s'", uid) |
There was a problem hiding this comment.
[nitpick] Instead of commenting out the log statement, consider using a more appropriate log level like log.Debug() if this is expected behavior, or remove the line entirely if the warning is no longer needed.
| // log.Warn().Msgf("No label found for '%s'", uid) | |
| log.Debug().Msgf("No label found for '%s'", uid) |
|
/approve |
No description provided.