feat/support layers #2061
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| -- Modify "digger_batches" table | ||
| ALTER TABLE "public"."digger_batches" ADD COLUMN "layer" bigint NULL; | ||
|
There was a problem hiding this comment. style: Consider using |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| h1:g7vBQEDCfExWRVQBcLZ9Jnxb1y/szNvlaeZjw1ZfRSY= | ||
| 20231227132525.sql h1:43xn7XC0GoJsCnXIMczGXWis9d504FAWi4F1gViTIcw= | ||
| 20240115170600.sql h1:IW8fF/8vc40+eWqP/xDK+R4K9jHJ9QBSGO6rN9LtfSA= | ||
| 20240116123649.sql h1:R1JlUIgxxF6Cyob9HdtMqiKmx/BfnsctTl5rvOqssQw= | ||
|
|
@@ -61,3 +61,4 @@ h1:DO/UG0tYCnZA69yRLMd2ZBKUORlJtZJLkVCiCogKp+k= | |
| 20250716222603.sql h1:a66teR/6BQwNwhJ/zgNA/NjUVTz5DcoQl7cDZvzVS8c= | ||
| 20250717032021.sql h1:HaIhNsz3C+c87CDmFjgFlc9zGqoE5BU4m0dofDpeDYk= | ||
| 20250725041417.sql h1:Dds6fqS415FD1jlsoVEahcEEm1px3EHV5435moL+Vp8= | ||
| 20250730002811.sql h1:VqNDMnhVinkBXlDJN4BCq5JzZr4NHVu6zuHEGP9dMvk= | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ const DiggerVCSBitbucket DiggerVCSType = "bitbucket" | |
| type DiggerBatch struct { | ||
| gorm.Model | ||
| ID uuid.UUID `gorm:"primary_key"` | ||
| Layer uint | ||
| VCS DiggerVCSType | ||
| PrNumber int | ||
| CommentId *int64 | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,6 +3,7 @@ package utils | |||||
| import ( | ||||||
| "fmt" | ||||||
| "log/slog" | ||||||
| "sort" | ||||||
|
|
||||||
| "github.com/diggerhq/digger/libs/ci" | ||||||
| "github.com/diggerhq/digger/libs/scheduler" | ||||||
|
|
@@ -135,6 +136,28 @@ func UpdateCRComment(cr *CommentReporter, comment string) error { | |||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| func trimMessageIfExceedsMaxLength(message string) string { | ||||||
|
|
||||||
| const GithubCommentMaxLength = 65536 | ||||||
|
|
||||||
| if len(message) > GithubCommentMaxLength { | ||||||
| slog.Warn("Comment message is too long, trimming", | ||||||
| "originalLength", len(message), | ||||||
| "maxLength", GithubCommentMaxLength, | ||||||
| ) | ||||||
|
|
||||||
| const footer = "[trimmed]" | ||||||
| trimLength := len(message) - GithubCommentMaxLength + len(footer) | ||||||
| message = message[:len(message)-trimLength] + footer | ||||||
|
|
||||||
| slog.Debug("Trimmed comment message", | ||||||
| "newLength", len(message), | ||||||
| "trimmedBytes", trimLength, | ||||||
| ) | ||||||
| } | ||||||
| return message | ||||||
| } | ||||||
|
|
||||||
| func ReportInitialJobsStatus(cr *CommentReporter, jobs []scheduler.Job) error { | ||||||
| prNumber := cr.PrNumber | ||||||
| prService := cr.PrService | ||||||
|
|
@@ -158,24 +181,7 @@ func ReportInitialJobsStatus(cr *CommentReporter, jobs []scheduler.Job) error { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| message = trimMessageIfExceedsMaxLength(message) | ||||||
| err := prService.EditComment(prNumber, commentId, message) | ||||||
| if err != nil { | ||||||
| slog.Error("Failed to update comment with initial jobs status", | ||||||
|
|
@@ -190,39 +196,61 @@ func ReportInitialJobsStatus(cr *CommentReporter, jobs []scheduler.Job) error { | |||||
| return nil | ||||||
| } | ||||||
|
|
||||||
| func ReportLayersTableForJobs(cr *CommentReporter, jobs []scheduler.Job) error { | ||||||
|
There was a problem hiding this comment. style: Function name suggests it reports layers table but the log message still says 'Reporting initial jobs status' - should be updated for consistency |
||||||
| prNumber := cr.PrNumber | ||||||
| prService := cr.PrService | ||||||
| commentId := cr.CommentId | ||||||
|
|
||||||
| // sort jobs by layer for better display (sort by name too) | ||||||
| sort.Slice(jobs, func(i, j int) bool { | ||||||
| if jobs[i].Layer == jobs[j].Layer { | ||||||
| return jobs[i].ProjectName < jobs[j].ProjectName | ||||||
| } | ||||||
| return jobs[i].Layer < jobs[j].Layer | ||||||
| }) | ||||||
|
|
||||||
| slog.Info("Reporting initial jobs status", | ||||||
|
There was a problem hiding this comment. style: Log message is inconsistent with function purpose - should say 'Reporting layers table' instead of 'initial jobs status'
Suggested change
Context Used: Context - Use consistent and descriptive keys for debug logging. For example, prefer 'prBatchCount' over 'len(prBatches)' to maintain clarity and adhere to naming conventions. (link) |
||||||
| "prNumber", prNumber, | ||||||
| "commentId", commentId, | ||||||
| "jobCount", len(jobs), | ||||||
| ) | ||||||
|
|
||||||
| message := "" | ||||||
| if len(jobs) == 0 { | ||||||
| message = message + ":construction_worker: No projects impacted" | ||||||
| } else { | ||||||
| message = message + fmt.Sprintf("| Project | Layer |\n") | ||||||
| message = message + fmt.Sprintf("|---------|--------|\n") | ||||||
| for _, job := range jobs { | ||||||
| message = message + fmt.Sprintf(""+ | ||||||
| "|:clock11: **%v**|%v|\n", job.ProjectName, job.Layer) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| message += "----------------\n\n" | ||||||
| message += ` | ||||||
| <details> | ||||||
| <summary>Instructions</summary> | ||||||
|
|
||||||
| Since you enabled layers in your configuration, you can proceed to perform a layer-by-layer deployment. | ||||||
| To start planning the first layer you can run "digger plan --layer 0". To apply the first layer, run "digger apply --layer 0". | ||||||
|
|
||||||
| To deploy the next layer, run "digger plan --layer 1". To apply the next layer, run "digger apply --layer 1". | ||||||
|
|
||||||
| And so on. A new commit on the branch will restart this deployment process. | ||||||
| </details> | ||||||
| ` | ||||||
| message = trimMessageIfExceedsMaxLength(message) | ||||||
| err := prService.EditComment(prNumber, commentId, message) | ||||||
| if err != nil { | ||||||
| slog.Error("Failed to update comment with initial jobs status", | ||||||
|
There was a problem hiding this comment. style: Error log message is generic 'initial jobs status' but should reflect the layers table context
Suggested change
Context Used: Context - Enrich debug logs with relevant context. Always include identifiers like 'prNumber' in logs to facilitate easier tracing and troubleshooting. (link) |
||||||
| "prNumber", prNumber, | ||||||
| "commentId", commentId, | ||||||
| "error", err, | ||||||
| ) | ||||||
| return err | ||||||
| } | ||||||
|
|
||||||
| slog.Debug("Successfully reported initial jobs status", "prNumber", prNumber, "commentId", commentId) | ||||||
|
There was a problem hiding this comment. style: Success log message should reflect the actual operation being performed (layers table) rather than generic 'initial jobs status'
Suggested change
Context Used: Context - Use consistent and descriptive keys for debug logging. For example, prefer 'prBatchCount' over 'len(prBatches)' to maintain clarity and adhere to naming conventions. (link) |
||||||
| return nil | ||||||
| } | ||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -921,7 +921,7 @@ func TestGitHubNewCommentContext(t *testing.T) { | |
| lock := &locking.MockLock{} | ||
| prManager := ci.MockPullRequestManager{ChangedFiles: []string{"dev/test.tf"}} | ||
| planStorage := storage.MockPlanStorage{} | ||
| impactedProjects, _, prNumber, err := dggithub.ProcessGitHubEvent(ghEvent, &diggerConfig, &prManager) | ||
| reporter := &reporting.CiReporter{ | ||
| CiService: &prManager, | ||
| PrNumber: prNumber, | ||
|
|
@@ -930,7 +930,7 @@ func TestGitHubNewCommentContext(t *testing.T) { | |
| policyChecker := policy.MockPolicyChecker{} | ||
| backendApi := backendapi.MockBackendApi{} | ||
|
|
||
| jobs, _, err := generic.ConvertIssueCommentEventToJobs("", "", 0, "", impactedProjects, impactedProjects, map[string]configuration.Workflow{}, "prbranch", "") | ||
| _, _, err = digger.RunJobs(jobs, &prManager, prManager, lock, reporter, &planStorage, policyChecker, comment_updater.NoopCommentUpdater{}, backendApi, "123", false, false, "1", "") | ||
| assert.NoError(t, err) | ||
| if err != nil { | ||
|
|
@@ -1020,10 +1020,9 @@ func TestGitHubTestPRCommandCaseInsensitivity(t *testing.T) { | |
| var impactedProjects []configuration.Project | ||
| impactedProjects = make([]configuration.Project, 1) | ||
| impactedProjects[0] = project | ||
| workflows := make(map[string]configuration.Workflow, 1) | ||
| workflows["default"] = configuration.Workflow{} | ||
| jobs, _, err := generic.ConvertIssueCommentEventToJobs("", "", 0, "digger plan", impactedProjects, impactedProjects, workflows, "prbranch", "main") | ||
|
|
||
| assert.Equal(t, 1, len(jobs)) | ||
| assert.Equal(t, "digger plan", jobs[0].Commands[0]) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -289,7 +289,14 @@ func GitHubCI(lock core_locking.Lock, policyCheckerProvider core_policy.PolicyCh | |
| repoFullName := *commentEvent.Repo.FullName | ||
| requestedBy := *commentEvent.Sender.Login | ||
| commentBody := *commentEvent.Comment.Body | ||
|
|
||
| var impactedProjectsForEvent []digger_config.Project | ||
| if requestedProject != nil { | ||
| impactedProjectsForEvent = []digger_config.Project{*requestedProject} | ||
| } else { | ||
| impactedProjectsForEvent = impactedProjects | ||
| } | ||
| jobs, coversAllImpactedProjects, err = generic.ConvertIssueCommentEventToJobs(repoFullName, requestedBy, prNumber, commentBody, impactedProjectsForEvent, impactedProjects, diggerConfig.Workflows, prBranchName, defaultBranch) | ||
| } else { | ||
| usage.ReportErrorAndExit(githubActor, fmt.Sprintf("Unsupported GitHub event type. %s", err), 6) | ||
| } | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current implementation of the
respect_layersfeature silently returns early when multiple layers are detected, without providing clear feedback to the user about why no jobs are being triggered. This can be confusing for users who expect automatic job triggering.The bug is that while the code does report a layers table with instructions in a collapsible details section, it doesn't explicitly inform users that no automatic job triggering will occur. This significant change in behavior should be prominently communicated.
The fix adds a more visible message explaining that when
respect_layersis enabled with multiple layers, jobs aren't automatically triggered, and directs users to the instructions for manually triggering jobs by layer. It also improves the logging from debug to info level for better visibility in logs.