-
Notifications
You must be signed in to change notification settings - Fork 563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
publish ai summaries #1864
publish ai summaries #1864
Changes from 2 commits
66376e1
6001a9d
2553102
d76db70
e9e2c78
af9d4eb
69b2db3
c08f33c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -511,7 +511,18 @@ func handlePullRequestEvent(gh utils.GithubClientProvider, payload *github.PullR | |
log.Printf("strconv.ParseInt error: %v", err) | ||
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not handle commentId: %v", err)) | ||
} | ||
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, models.DiggerVCSGithub, organisationId, impactedJobsMap, impactedProjectsMap, projectsGraph, installationId, branch, prNumber, repoOwner, repoName, repoFullName, commitSha, commentId, diggerYmlStr, 0) | ||
|
||
var aiSummaryCommentId = "" | ||
if config.Reporting.AiSummary { | ||
aiSummaryComment, err := ghService.PublishComment(prNumber, "AI Summary will be posted here after completion") | ||
if err != nil { | ||
log.Printf("could not post ai summary comment: %v", err) | ||
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not post ai comment summary comment id: %v", err)) | ||
} | ||
aiSummaryCommentId = aiSummaryComment.Id | ||
} | ||
|
||
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, models.DiggerVCSGithub, organisationId, impactedJobsMap, impactedProjectsMap, projectsGraph, installationId, branch, prNumber, repoOwner, repoName, repoFullName, commitSha, commentId, diggerYmlStr, 0, aiSummaryCommentId, config.ReportTerraformOutputs) | ||
if err != nil { | ||
log.Printf("ConvertJobsToDiggerJobs error: %v", err) | ||
commentReporterManager.UpdateComment(fmt.Sprintf(":x: ConvertJobsToDiggerJobs error: %v", err)) | ||
|
@@ -902,7 +913,17 @@ func handleIssueCommentEvent(gh utils.GithubClientProvider, payload *github.Issu | |
return fmt.Errorf("comment reporter error: %v", err) | ||
} | ||
|
||
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, "github", orgId, impactedProjectsJobMap, impactedProjectsMap, projectsGraph, installationId, *branch, issueNumber, repoOwner, repoName, repoFullName, *commitSha, reporterCommentId, diggerYmlStr, 0) | ||
var aiSummaryCommentId = "" | ||
if config.Reporting.AiSummary { | ||
aiSummaryComment, err := ghService.PublishComment(issueNumber, "AI Summary will be posted here after completion") | ||
if err != nil { | ||
log.Printf("could not post ai summary comment: %v", err) | ||
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not post ai comment summary comment id: %v", err)) | ||
} | ||
aiSummaryCommentId = aiSummaryComment.Id | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check for nil after failing to publish the AI summary comment. Similarly to the earlier comment, if Proposed fix: var aiSummaryCommentId = ""
if config.Reporting.AiSummary {
aiSummaryComment, err := ghService.PublishComment(issueNumber, "AI Summary will be posted here after completion")
if err != nil {
log.Printf("could not post ai summary comment: %v", err)
commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not post ai comment summary comment id: %v", err))
+ return
}
aiSummaryCommentId = aiSummaryComment.Id
}
|
||
batchId, _, err := utils.ConvertJobsToDiggerJobs(*diggerCommand, "github", orgId, impactedProjectsJobMap, impactedProjectsMap, projectsGraph, installationId, *branch, issueNumber, repoOwner, repoName, repoFullName, *commitSha, reporterCommentId, diggerYmlStr, 0, aiSummaryCommentId, config.ReportTerraformOutputs) | ||
if err != nil { | ||
log.Printf("ConvertJobsToDiggerJobs error: %v", err) | ||
commentReporterManager.UpdateComment(fmt.Sprintf(":x: ConvertJobsToDiggerJobs error: %v", err)) | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -17,6 +17,7 @@ import ( | |||||||||||||||||||||||
"gorm.io/gorm" | ||||||||||||||||||||||||
"log" | ||||||||||||||||||||||||
"net/http" | ||||||||||||||||||||||||
"os" | ||||||||||||||||||||||||
"strconv" | ||||||||||||||||||||||||
"strings" | ||||||||||||||||||||||||
"time" | ||||||||||||||||||||||||
|
@@ -323,7 +324,6 @@ type SetJobStatusRequest struct { | |||||||||||||||||||||||
Footprint *iac_utils.IacPlanFootprint `json:"job_plan_footprint"` | ||||||||||||||||||||||||
PrCommentUrl string `json:"pr_comment_url"` | ||||||||||||||||||||||||
TerraformOutput string `json:"terraform_output"` | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
func (d DiggerController) SetJobStatusForProject(c *gin.Context) { | ||||||||||||||||||||||||
|
@@ -488,6 +488,12 @@ func (d DiggerController) SetJobStatusForProject(c *gin.Context) { | |||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "Error merging PR with automerge option"}) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
err = CreateTerraformPlansSummary(d.GithubClientProvider, batch) | ||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||
log.Printf("could not generate terraform plans summary: %v", err) | ||||||||||||||||||||||||
c.JSON(http.StatusInternalServerError, gin.H{"error": "could not generate terraform plans summary"}) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Improve error handling for CreateTerraformPlansSummary The current implementation continues execution after logging the error and sending a 500 response. This could lead to multiple status codes being sent if subsequent operations fail. Consider handling the error more gracefully: err = CreateTerraformPlansSummary(d.GithubClientProvider, batch)
if err != nil {
log.Printf("could not generate terraform plans summary: %v", err)
- c.JSON(http.StatusInternalServerError, gin.H{"error": "could not generate terraform plans summary"})
+ // Log error but continue as this is not a critical operation
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||
// return batch summary to client | ||||||||||||||||||||||||
res, err := batch.MapToJsonStruct() | ||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||
|
@@ -648,6 +654,58 @@ func GetPrServiceFromBatch(batch *models.DiggerBatch, gh utils.GithubClientProvi | |||||||||||||||||||||||
return nil, fmt.Errorf("could not retrieive a service for %v", batch.VCS) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
func CreateTerraformPlansSummary(gh utils.GithubClientProvider, batch *models.DiggerBatch) error { | ||||||||||||||||||||||||
diggerYmlString := batch.DiggerConfig | ||||||||||||||||||||||||
diggerConfigYml, err := digger_config.LoadDiggerConfigYamlFromString(diggerYmlString) | ||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||
log.Printf("Error loading digger config from batch: %v", err) | ||||||||||||||||||||||||
return fmt.Errorf("error loading digger config from batch: %v", err) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
config, _, err := digger_config.ConvertDiggerYamlToConfig(diggerConfigYml) | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if batch.Status == orchestrator_scheduler.BatchJobSucceeded && config.Reporting.AiSummary == true { | ||||||||||||||||||||||||
prService, err := GetPrServiceFromBatch(batch, gh) | ||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||
log.Printf("Error getting github service: %v", err) | ||||||||||||||||||||||||
return fmt.Errorf("error getting github service: %v", err) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
summaryEndpoint := os.Getenv("DIGGER_AI_SUMMARY_ENDPOINT") | ||||||||||||||||||||||||
if summaryEndpoint == "" { | ||||||||||||||||||||||||
log.Printf("could not generate AI summary, ai summary endpoint missing") | ||||||||||||||||||||||||
return fmt.Errorf("could not generate AI summary, ai summary endpoint missing") | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
apiToken := os.Getenv("DIGGER_AI_SUMMARY_API_TOKEN") | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
motatoes marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
jobs, err := models.DB.GetDiggerJobsForBatch(batch.ID) | ||||||||||||||||||||||||
var terraformOutputs = "" | ||||||||||||||||||||||||
for _, job := range jobs { | ||||||||||||||||||||||||
var jobSpec orchestrator_scheduler.JobJson | ||||||||||||||||||||||||
err := json.Unmarshal(job.SerializedJobSpec, &jobSpec) | ||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||
log.Printf("could not summarise plans due to unmarshalling error: %v", err) | ||||||||||||||||||||||||
return fmt.Errorf("could not summarise plans due to unmarshalling error: %v", err) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
projectName := jobSpec.ProjectName | ||||||||||||||||||||||||
terraformOutputs += fmt.Sprintf("terraform output for %v \n\n", projectName) + job.TerraformOutput | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
summary, err := utils.GetAiSummaryFromTerraformPlans(terraformOutputs, summaryEndpoint, apiToken) | ||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||
log.Printf("could not summarise terraform outputs: %v", err) | ||||||||||||||||||||||||
return fmt.Errorf("could not summarise terraform outputs: %v", err) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
motatoes marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||
|
||||||||||||||||||||||||
if batch.AiSummaryCommentId == "" { | ||||||||||||||||||||||||
log.Printf("could not post summary comment, initial comment not found") | ||||||||||||||||||||||||
return fmt.Errorf("could not post summary comment, initial comment not found") | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
prService.EditComment(batch.PrNumber, batch.AiSummaryCommentId, summary) | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
return nil | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
func AutomergePRforBatchIfEnabled(gh utils.GithubClientProvider, batch *models.DiggerBatch) error { | ||||||||||||||||||||||||
diggerYmlString := batch.DiggerConfig | ||||||||||||||||||||||||
diggerConfigYml, err := digger_config.LoadDiggerConfigYamlFromString(diggerYmlString) | ||||||||||||||||||||||||
|
@@ -663,13 +721,6 @@ func AutomergePRforBatchIfEnabled(gh utils.GithubClientProvider, batch *models.D | |||||||||||||||||||||||
automerge = false | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
if batch.Status == orchestrator_scheduler.BatchJobSucceeded && batch.BatchType == orchestrator_scheduler.DiggerCommandApply && automerge == true { | ||||||||||||||||||||||||
//ghService, _, err := utils.GetGithubService( | ||||||||||||||||||||||||
// gh, | ||||||||||||||||||||||||
// batch.GithubInstallationId, | ||||||||||||||||||||||||
// batch.RepoFullName, | ||||||||||||||||||||||||
// batch.RepoOwner, | ||||||||||||||||||||||||
// batch.RepoName, | ||||||||||||||||||||||||
//) | ||||||||||||||||||||||||
prService, err := GetPrServiceFromBatch(batch, gh) | ||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||
log.Printf("Error getting github service: %v", err) | ||||||||||||||||||||||||
|
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.
Check for nil after failing to publish the AI summary comment.
If
PublishComment
fails, we log the error but still proceed to useaiSummaryComment.Id
, which may cause a nil pointer dereference. Consider returning early when an error occurs.Proposed fix:
var aiSummaryCommentId = "" if config.Reporting.AiSummary { aiSummaryComment, err := ghService.PublishComment(prNumber, "AI Summary will be posted here after completion") if err != nil { log.Printf("could not post ai summary comment: %v", err) commentReporterManager.UpdateComment(fmt.Sprintf(":x: could not post ai comment summary comment id: %v", err)) + return } aiSummaryCommentId = aiSummaryComment.Id }