Revert "cancel jobs" #75

alephys26 · 2025-12-29T08:05:20Z

Reverts #64 due to errors downstream.
It will be merged again after the necessary fixes are made.

This reverts commit 555b316.

wiz-55ccc8b716 · 2025-12-29T08:06:09Z

Wiz Scan Summary

Scanner	Findings
Vulnerabilities	-
Sensitive Data	-
Secrets	-
IaC Misconfigurations	-
SAST Findings	2
Software Supply Chain Findings	-

Total	2

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

Copilot

Pull request overview

This PR reverts the "cancel jobs" feature (#64) that was previously merged but caused errors downstream. The revert removes job cancellation functionality including the cancellation statuses, API endpoints, database fields, and context-based cancellation monitoring.

Key Changes:

Removes job cancellation API endpoint and related database operations
Reverts job status constants from 8 statuses (including CANCELLING and CANCELLED) back to 6 statuses
Removes cancelled_by field from job structures and database schema
Simplifies plugin handler signatures by removing context.Context parameter and using global context where needed

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
web/src/modules/Jobs/Helper.tsx	Removes `cancelled_by` field from TypeScript JobType interface
pkg/plugin/runtime.go	Reverts WriteToS3 function call to direct reference without context wrapper
pkg/plugin/plugin.go	Simplifies Handler signature by removing context.Context parameter
pkg/object/job/status/status.go	Removes CANCELLING and CANCELLED status constants, reducing from 8 to 6 statuses
pkg/object/job/job.go	Removes `CancelledBy` field from Job struct (introduces typo: "CluserID")
internal/pkg/pool/pool.go	Simplifies worker function signature by removing context parameter
internal/pkg/object/command/trino/*	Removes context parameter from handler and client functions
internal/pkg/object/command/sparkeks/*	Removes context parameter, adds global ctx variable, renames types with sparkEks prefix
internal/pkg/object/command/spark/*	Removes context parameter, adds global ctx variable, renames types with spark prefix
internal/pkg/object/command/snowflake/*	Removes context parameter from handlers, renames types with snowflake prefix
internal/pkg/object/command/shell/*	Removes context parameter from handlers, renames types with shell prefix
internal/pkg/object/command/ping/*	Removes context parameter from handlers, renames types with ping prefix
internal/pkg/object/command/glue/*	Removes context parameter from handlers, renames types with glue prefix
internal/pkg/object/command/ecs/*	Removes context parameter, adds global ctx variable, renames types with ecs prefix
internal/pkg/object/command/dynamo/*	Removes context parameter, adds global ctx variable, renames types with dynamo prefix
internal/pkg/object/command/clickhouse/*	Removes context parameter from handler, adds local ctx variable
internal/pkg/heimdall/queries/*	Removes status_cancel_update.sql query and reverts select queries to remove cancelled_by column
internal/pkg/heimdall/jobs_async.go	Updates to use typo'd field name, adds job status determination logic
internal/pkg/heimdall/job_dal.go	Removes User field from jobRequest, updates queries to remove cancelled_by
internal/pkg/heimdall/job.go	Removes cancelJob function and cancellation monitoring logic, simplifies runJob execution
internal/pkg/heimdall/heimdall.go	Removes /job/{id}/cancel endpoint, reorders imports
internal/pkg/heimdall/handler.go	Removes context.Context parameter from payloadHandler signature
internal/pkg/heimdall/command_dal.go	Removes context parameter from command DAL functions
internal/pkg/heimdall/cluster_dal.go	Removes context parameter from cluster DAL functions
internal/pkg/aws/s3.go	Removes context parameter from S3 functions, adds global ctx variable
internal/pkg/aws/glue.go	Removes context parameter from Glue functions
internal/pkg/aws/cloudwatch.go	Removes context parameter from PullLogs function (but still uses undefined ctx)
assets/databases/heimdall/tables/jobs.sql	Removes cancelled_by column alteration statements
assets/databases/heimdall/data/job_statuses.sql	Removes CANCELLING and CANCELLED status entries
README.md	Updates plugin table formatting and removes cancel endpoint from API documentation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-29T08:13:44Z

internal/pkg/aws/cloudwatch.go

 )

-func PullLogs(ctx context.Context, writer *os.File, logGroup, logStream string, chunkSize int, memoryLimit int64) error {
+func PullLogs(writer *os.File, logGroup, logStream string, chunkSize int, memoryLimit int64) error {


The context import was removed but the function still uses ctx on lines 16 and 41. The ctx variable is not defined anywhere in this file, which will cause a compilation error. A global context variable should be defined similar to other files in this PR, or the context parameter should be kept in the function signature.

Copilot · 2025-12-29T08:13:45Z

internal/pkg/heimdall/heimdall.go

 	"github.com/patterninc/heimdall/internal/pkg/server"
 	"github.com/patterninc/heimdall/pkg/object/cluster"
 	"github.com/patterninc/heimdall/pkg/object/command"
 	"github.com/patterninc/heimdall/pkg/object/job"
 	"github.com/patterninc/heimdall/pkg/plugin"
+	"github.com/patterninc/heimdall/internal/pkg/rbac"


The import order has been changed inconsistently. The rbac import was moved from line 17 (before server) to line 23 (after plugin). This breaks the conventional Go import grouping where internal packages should be grouped together. The import should remain in its original position before the server import.

Suggested change

"github.com/patterninc/heimdall/internal/pkg/server"

"github.com/patterninc/heimdall/pkg/object/cluster"

"github.com/patterninc/heimdall/pkg/object/command"

"github.com/patterninc/heimdall/pkg/object/job"

"github.com/patterninc/heimdall/pkg/plugin"

"github.com/patterninc/heimdall/internal/pkg/rbac"

"github.com/patterninc/heimdall/internal/pkg/rbac"

"github.com/patterninc/heimdall/internal/pkg/server"

"github.com/patterninc/heimdall/pkg/object/cluster"

"github.com/patterninc/heimdall/pkg/object/command"

"github.com/patterninc/heimdall/pkg/object/job"

"github.com/patterninc/heimdall/pkg/plugin"

Copilot · 2025-12-29T08:13:45Z

internal/pkg/object/command/trino/trino.go

 		}
 	}
-
+	


This line contains only whitespace (trailing spaces or tabs). It should be completely empty with no trailing whitespace.

Suggested change

Copilot · 2025-12-29T08:13:45Z

internal/pkg/heimdall/jobs_async.go

 		jobContext, j := ``, &job.Job{}

-		if err := rows.Scan(&j.SystemID, &j.CommandID, &j.ClusterID, &j.Status, &j.ID, &j.Name,
+		if err := rows.Scan(&j.SystemID, &j.CommandID, &j.CluserID, &j.Status, &j.ID, &j.Name,


The typo "CluserID" (should be "ClusterID") propagates to the Scan operation. This will cause the field mapping to fail when scanning database rows into the job struct.

Suggested change

if err := rows.Scan(&j.SystemID, &j.CommandID, &j.CluserID, &j.Status, &j.ID, &j.Name,

if err := rows.Scan(&j.SystemID, &j.CommandID, &j.ClusterID, &j.Status, &j.ID, &j.Name,

Copilot · 2025-12-29T08:13:46Z

internal/pkg/heimdall/jobs_async.go

 	defer updateAsyncJobStatusMethod.RecordLatency(time.Now())
 	updateAsyncJobStatusMethod.CountRequest()

+	// we updte the final job status based on presence of the error


Spelling error in the comment: "updte" should be "update".

Copilot · 2025-12-29T08:13:46Z

pkg/object/job/job.go

 	CommandID       string           `yaml:"command_id,omitempty" json:"command_id,omitempty"`
 	CommandName     string           `yaml:"command_name,omitempty" json:"command_name,omitempty"`
-	ClusterID       string           `yaml:"cluster_id,omitempty" json:"cluster_id,omitempty"`
+	CluserID        string           `yaml:"cluster_id,omitempty" json:"cluster_id,omitempty"`


There is a typo in the field name. The field is named "CluserID" but it should be "ClusterID" (missing 't'). This typo affects multiple files in this PR and needs to be corrected.

Copilot · 2025-12-29T08:13:47Z

internal/pkg/heimdall/jobs_async.go


 	// do we have hte cluster?
-	cluster, found := h.Clusters[j.ClusterID]
+	cluster, found := h.Clusters[j.CluserID]


The typo "CluserID" (should be "ClusterID") is used here to reference the cluster from the job. This will result in referencing the wrong field and likely cause runtime errors when trying to access the cluster.

Copilot · 2025-12-29T08:13:47Z

internal/pkg/heimdall/jobs_async.go

+	cluster, found := h.Clusters[j.CluserID]
 	if !found {
-		return h.updateAsyncJobStatus(j, fmt.Errorf(formatErrUnknownCluster, j.ClusterID))
+		return h.updateAsyncJobStatus(j, fmt.Errorf(formatErrUnknownCluster, j.CluserID))


The typo "CluserID" (should be "ClusterID") is used in the error message format string. This will reference the wrong field and display incorrect or empty cluster information in error messages.

Copilot · 2025-12-29T08:13:47Z

internal/pkg/heimdall/job_dal.go


 	if err := row.Scan(&r.SystemID, &r.Status, &r.Name, &r.Version, &r.Description, &jobContext, &r.Error, &r.User, &r.IsSync,
-		&r.CreatedAt, &r.UpdatedAt, &r.CommandID, &r.CommandName, &r.ClusterID, &r.ClusterName, &r.StoreResultSync, &r.CancelledBy); err != nil {
+		&r.CreatedAt, &r.UpdatedAt, &r.CommandID, &r.CommandName, &r.CluserID, &r.ClusterName, &r.StoreResultSync); err != nil {


The typo "CluserID" (should be "ClusterID") is used in the Scan operation. This will cause the field mapping to fail when scanning database rows into the job struct.

Suggested change

&r.CreatedAt, &r.UpdatedAt, &r.CommandID, &r.CommandName, &r.CluserID, &r.ClusterName, &r.StoreResultSync); err != nil {

&r.CreatedAt, &r.UpdatedAt, &r.CommandID, &r.CommandName, &r.ClusterID, &r.ClusterName, &r.StoreResultSync); err != nil {

Copilot · 2025-12-29T08:13:47Z

internal/pkg/heimdall/job_dal.go


 		if err := rows.Scan(&r.SystemID, &r.ID, &r.Status, &r.Name, &r.Version, &r.Description, &jobContext, &r.Error, &r.User, &r.IsSync,
-			&r.CreatedAt, &r.UpdatedAt, &r.CommandID, &r.CommandName, &r.ClusterID, &r.ClusterName, &r.StoreResultSync, &r.CancelledBy); err != nil {
+			&r.CreatedAt, &r.UpdatedAt, &r.CommandID, &r.CommandName, &r.CluserID, &r.ClusterName, &r.StoreResultSync); err != nil {


The typo "CluserID" (should be "ClusterID") is used in the Scan operation. This will cause the field mapping to fail when scanning database rows into the job struct.

Suggested change

&r.CreatedAt, &r.UpdatedAt, &r.CommandID, &r.CommandName, &r.CluserID, &r.ClusterName, &r.StoreResultSync); err != nil {

&r.CreatedAt, &r.UpdatedAt, &r.CommandID, &r.CommandName, &r.ClusterID, &r.ClusterName, &r.StoreResultSync); err != nil {

Revert "cancel jobs (#64)"

806c813

This reverts commit 555b316.

alephys26 requested review from Copilot and sanketjadhavSF December 29, 2025 08:05

Copilot started reviewing on behalf of alephys26 December 29, 2025 08:05 View session

Copilot AI reviewed Dec 29, 2025

View reviewed changes

alephys26 closed this Dec 29, 2025

alephys26 deleted the revert-64-cancel_jobs branch December 29, 2025 10:15

	if err := rows.Scan(&j.SystemID, &j.CommandID, &j.CluserID, &j.Status, &j.ID, &j.Name,
	if err := rows.Scan(&j.SystemID, &j.CommandID, &j.ClusterID, &j.Status, &j.ID, &j.Name,

	&r.CreatedAt, &r.UpdatedAt, &r.CommandID, &r.CommandName, &r.CluserID, &r.ClusterName, &r.StoreResultSync); err != nil {
	&r.CreatedAt, &r.UpdatedAt, &r.CommandID, &r.CommandName, &r.ClusterID, &r.ClusterName, &r.StoreResultSync); err != nil {

Revert "cancel jobs" #75

Revert "cancel jobs" #75

Uh oh!

Conversation

alephys26 commented Dec 29, 2025

Uh oh!

wiz-55ccc8b716 bot commented Dec 29, 2025

Wiz Scan Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants