Refactor mci dynamic codes#2117
Conversation
Signed-off-by: Seokho Son <shsongist@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR refactors MCI (Multi-Cloud Infrastructure) dynamic codes by adding a new parameter isReqFromDynamic to the CreateMci function to distinguish between manual and dynamic MCI creation requests. The changes improve the handling of dynamic MCI creation workflows and enhance error management.
Key changes:
- Added
isReqFromDynamicparameter toCreateMcifunction signature - Reorganized MCI creation logic to handle dynamic requests differently from manual ones
- Improved error handling with better validation error formatting and cleanup
- Moved
CreateSystemMciDynamicfunction location for better code organization
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/interface/rest/server/infra/provisioning.go | Updated REST API calls to pass false for the new isReqFromDynamic parameter |
| src/core/infra/utility.go | Updated utility function call to include the new parameter |
| src/core/infra/provisioning.go | Main refactoring: added new parameter, reorganized MCI creation logic, improved error handling, and moved function location |
| src/core/infra/manageInfo.go | Minor logging improvement for MCI termination waiting |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
| if mciStatusTmp.Status == model.StatusPrepared { | ||
| // starting status for subgroups | ||
| log.Error().Err(err).Msgf("MCI '%s' does not exist in namespace '%s' should be prepared by dynamic request", mciId, nsId) |
There was a problem hiding this comment.
There's a grammatical error in the error message. It should read 'should be prepared by a dynamic request' or 'should have been prepared by dynamic request'.
| log.Error().Err(err).Msgf("MCI '%s' does not exist in namespace '%s' should be prepared by dynamic request", mciId, nsId) | |
| log.Error().Err(err).Msgf("MCI '%s' does not exist in namespace '%s'; should have been prepared by dynamic request", mciId, nsId) |
| return nil, fmt.Errorf("MCI '%s' already exists in namespace '%s'", mciId, nsId) | ||
| } else { | ||
| req.SystemLabel = "Registered from CSP resource" | ||
| req.SystemLabel = "Registered from CSP" |
There was a problem hiding this comment.
[nitpick] The system label was changed from 'Registered from CSP resource' to 'Registered from CSP'. This appears to be an intentional simplification, but consider whether the more specific label provided better context for debugging and auditing purposes.
| req.SystemLabel = "Registered from CSP" | |
| req.SystemLabel = "Registered from CSP resource" |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
/approve |
No description provided.