Skip to content

Commit

Permalink
fix: stop jobs with short ids (#4657)
Browse files Browse the repository at this point in the history
This PR enables stopping jobs using short IDs, such as `j-d43c044b`
instead of `j-d43c044b-f731-4256-a3d6-e62fac426d15`. It also improves
the error reported when passing wrong jobID or if multiple jobs match
the pefix

### Testing Done
- Introduced new `StopSuite` test suite
- Manual testing as shown below
```
# short job id
→ bacalhau job stop j-3dc57b6a
Checking job status

	Connecting to network  ................  done ✅  0.0s
	  Verifying job state  ................  done ✅  0.0s
	         Stopping job  ................  done ✅  0.0s

Job stop successfully submitted with evaluation ID: 65035d30-b093-4458-bd8e-ac5b5aad27a4


# multiple jobs matching the prefix
→ bacalhau job stop j-
Checking job status

	Connecting to network  ................  done ✅  0.0s
	  Verifying job state  ................  err  ❌  0.0s

Error: multiple jobs found for id j-
Hint:  Use full job ID


# no matching job id
→ bacalhau job stop j-123
Checking job status

	Connecting to network  ................  done ✅  0.0s
	  Verifying job state  ................  err  ❌  0.0s

Error: job not found: j-123

```

Fixes #4643 
Fixes #3674 
Fixes #4644
  • Loading branch information
wdbaruni authored Oct 24, 2024
1 parent 8996d60 commit fb0232a
Show file tree
Hide file tree
Showing 8 changed files with 849 additions and 120 deletions.
28 changes: 14 additions & 14 deletions pkg/orchestrator/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,18 @@ func (e *BaseEndpoint) SubmitJob(ctx context.Context, request *SubmitJobRequest)
}

func (e *BaseEndpoint) StopJob(ctx context.Context, request *StopJobRequest) (StopJobResponse, error) {
job, err := e.store.GetJob(ctx, request.JobID)
txContext, err := e.store.BeginTx(ctx)
if err != nil {
return StopJobResponse{}, jobstore.NewJobStoreError(err.Error())
}

defer func() {
if err != nil {
_ = txContext.Rollback()
}
}()

job, err := e.store.GetJob(txContext, request.JobID)
if err != nil {
return StopJobResponse{}, err
}
Expand All @@ -124,21 +135,10 @@ func (e *BaseEndpoint) StopJob(ctx context.Context, request *StopJobRequest) (St
// continue
}

txContext, err := e.store.BeginTx(ctx)
if err != nil {
return StopJobResponse{}, jobstore.NewJobStoreError(err.Error())
}

defer func() {
if err != nil {
_ = txContext.Rollback()
}
}()

// update the job state, except if the job is already completed
// we allow marking a failed job as canceled
if err = e.store.UpdateJobState(txContext, jobstore.UpdateJobStateRequest{
JobID: request.JobID,
JobID: job.ID, // use the job ID from the store in case the request had a short ID
Condition: jobstore.UpdateJobCondition{
UnexpectedStates: []models.JobStateType{
models.JobStateTypeCompleted,
Expand All @@ -161,7 +161,7 @@ func (e *BaseEndpoint) StopJob(ctx context.Context, request *StopJobRequest) (St
now := time.Now().UTC().UnixNano()
eval := &models.Evaluation{
ID: uuid.NewString(),
JobID: request.JobID,
JobID: job.ID,
TriggeredBy: models.EvalTriggerJobCancel,
Type: job.Type,
Status: models.EvalStatusPending,
Expand Down
17 changes: 16 additions & 1 deletion pkg/publicapi/apimodels/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,20 @@ type APIError struct {

// Component is the component that caused the error.
Component string `json:"Component"`

// Hint is a string providing additional context or suggestions related to the error.
Hint string `json:"Hint,omitempty"`

// Details is a map of string key-value pairs providing additional error context.
Details map[string]string `json:"Details,omitempty"`
}

// NewAPIError creates a new APIError with the given HTTP status code and message.
func NewAPIError(statusCode int, message string) *APIError {
return &APIError{
HTTPStatusCode: statusCode,
Message: message,
Details: make(map[string]string),
}
}

Expand Down Expand Up @@ -106,14 +113,22 @@ func FromBacError(err bacerrors.Error) *APIError {
Message: err.Error(),
Code: string(err.Code()),
Component: err.Component(),
Hint: err.Hint(),
Details: err.Details(),
}
}

// ToBacError converts an APIError to a bacerror.Error
func (e *APIError) ToBacError() bacerrors.Error {
details := e.Details
if details == nil {
details = make(map[string]string)
}
details["request_id"] = e.RequestID
return bacerrors.New(e.Error()).
WithHTTPStatusCode(e.HTTPStatusCode).
WithCode(bacerrors.Code(e.Code)).
WithComponent(e.Component).
WithDetails(map[string]string{"request_id": e.RequestID})
WithHint(e.Hint).
WithDetails(details)
}
Loading

0 comments on commit fb0232a

Please sign in to comment.