fix: deploy/delete deps try to connect to later stages#557
Conversation
Code Review Agent Run #fe3c3fActionable Suggestions - 0Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help AI Code Review powered by |
Signed-off-by: Ilya Lesikov <ilya@lesikov.com>
fdc017d to
f466e15
Compare
There was a problem hiding this comment.
Code Review Agent Run #16fded
Actionable Suggestions - 3
-
internal/plan/plan_build.go - 3
- Incorrect max iteration selection · Line 724-724
- Incorrect max iteration selection · Line 746-746
- Incorrect max iteration selection · Line 778-778
Review Details
-
Files reviewed - 1 · Commit Range:
f466e15..f466e15- internal/plan/plan_build.go
-
Files skipped - 1
- CONTRIBUTING.md - Reason: Filter setting
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- Golangci-lint (Linter) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at ilya.lesikov@flant.com.
Documentation & Help
AI Code Review powered by
| if candidate.MustInstall == ResourceInstallTypeNone || | ||
| candidate.Stage != sourceStage || | ||
| !dep.Match(candidate.ResourceMeta) || | ||
| (match != nil && candidate.Iteration >= match.Iteration) { |
There was a problem hiding this comment.
The condition skips candidates with higher iterations, potentially selecting the wrong operation for dependencies. This could lead to incorrect deployment order if multiple iterations exist.
Code suggestion
Check the AI-generated fix before applying
- for _, candidate := range instInfos {
- if candidate.MustInstall == ResourceInstallTypeNone ||
- candidate.Stage != sourceStage ||
- !dep.Match(candidate.ResourceMeta) ||
- (match != nil && candidate.Iteration >= match.Iteration) {
- continue
- }
-
- match = candidate
- }
+ for _, candidate := range instInfos {
+ if candidate.MustInstall == ResourceInstallTypeNone ||
- candidate.Stage != sourceStage ||
- !dep.Match(candidate.ResourceMeta) {
- continue
- }
-
- if match == nil || candidate.Iteration > match.Iteration {
- match = candidate
- }
- }
Code Review Run #16fded
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
| !dep.Match(candidate.ResourceMeta) || | ||
| (match != nil && candidate.Iteration >= match.Iteration) { | ||
| continue | ||
| } |
There was a problem hiding this comment.
The condition skips candidates with higher iterations, potentially selecting the wrong operation for readiness tracking dependencies. This could lead to incorrect deployment order if multiple iterations exist.
Code suggestion
Check the AI-generated fix before applying
- func findTrackReadinessOpInStage(plan *Plan, instInfos []*InstallableResourceInfo, dep *resource.InternalDependency, sourceStage common.Stage) (*Operation, bool) {
- var match *InstallableResourceInfo
- for _, candidate := range instInfos {
- if !candidate.MustTrackReadiness ||
- candidate.Stage != sourceStage ||
- !dep.Match(candidate.ResourceMeta) ||
- (match != nil && candidate.Iteration >= match.Iteration) {
- continue
- }
-
- match = candidate
- }
-
- if match == nil {
- return nil, false
- }
-
- topID := OperationID(OperationTypeTrackReadiness, OperationVersionTrackReadiness, OperationIteration(match.Iteration), match.ID())
-
- return plan.Operation(opID)
- }
+ func findTrackReadinessOpInStage(plan *Plan, instInfos []*InstallableResourceInfo, dep *resource.InternalDependency, sourceStage common.Stage) (*Operation, bool) {
+ var match *InstallableResourceInfo
+ for _, candidate := range instInfos {
+ if !candidate.MustTrackReadiness ||
- candidate.Stage != sourceStage ||
- !dep.Match(candidate.ResourceMeta) {
- continue
- }
-
- if match == nil || candidate.Iteration > match.Iteration {
- match = candidate
- }
- }
-
- if match == nil {
- return nil, false
- }
-
- opID := OperationID(OperationTypeTrackReadiness, OperationVersionTrackReadiness, OperationIteration(match.Iteration), match.ID())
-
- return plan.Operation(opID)
- }
Code Review Run #16fded
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
| if !candidate.MustDeleteOnSuccessfulInstall || | ||
| candidate.StageDeleteOnSuccessfulInstall != sourceStage || | ||
| !dep.Match(candidate.ResourceMeta) || | ||
| (match != nil && candidate.Iteration >= match.Iteration) { |
There was a problem hiding this comment.
The condition skips candidates with higher iterations, potentially selecting the wrong operation for absence tracking dependencies. This could lead to incorrect deployment order if multiple iterations exist.
Code suggestion
Check the AI-generated fix before applying
- if !candidate.MustDeleteOnSuccessfulInstall ||
- candidate.StageDeleteOnSuccessfulInstall != sourceStage ||
- !dep.Match(candidate.ResourceMeta) ||
- (match != nil && candidate.Iteration >= match.Iteration) {
- continue
- }
-
- match = candidate
+ if !candidate.MustDeleteOnSuccessfulInstall ||
- candidate.StageDeleteOnSuccessfulInstall != sourceStage ||
- !dep.Match(candidate.ResourceMeta) {
- continue
- }
-
- if match == nil || candidate.Iteration > match.Iteration {
- match = candidate
- }
Code Review Run #16fded
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
Summary by Bito
This PR fixes a bug in the plan building process where deploy and delete dependency connections were attempting to link to operations in later stages, which could lead to incorrect execution order and potential failures in resource management. The modifications ensure dependencies are resolved within the same stage, enhancing the reliability of multi-stage deployments.
Detailed Changes