Skip to content
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

Show Rejections in Lookout #3663

Merged
merged 16 commits into from
Jun 24, 2024
Merged

Show Rejections in Lookout #3663

merged 16 commits into from
Jun 24, 2024

Conversation

d80tb7
Copy link
Collaborator

@d80tb7 d80tb7 commented Jun 9, 2024

Backend changes necessary to show rejection messages in lookout. Changes here are:

  • Adds a new REJECTED state to lookout
  • Adds a new job_error table to the lookout db
  • Jobs are marked as REJECTED if the event failure message is of type Error_JobRejected. We also store the error in the job_error table
  • New Repository as been added which allows a job error to be looked up by jobId.

We shouldn't merge this until the frontend code is ready.

d80tb7 added 8 commits June 9, 2024 09:55
Signed-off-by: Chris Martin <[email protected]>
Signed-off-by: Chris Martin <[email protected]>
Signed-off-by: Chris Martin <[email protected]>
Signed-off-by: Chris Martin <[email protected]>
Signed-off-by: Chris Martin <[email protected]>
Signed-off-by: Chris Martin <[email protected]>
Signed-off-by: Chris Martin <[email protected]>
Signed-off-by: Chris Martin <[email protected]>
@d80tb7 d80tb7 changed the title [WIP] Show Rejections in Lookout [WIP] Show Rejections in Lookout Jun 11, 2024
@d80tb7 d80tb7 added the do-not-merge Do not merge this PR label Jun 11, 2024
@d80tb7 d80tb7 changed the title [WIP] Show Rejections in Lookout Show Rejections in Lookout Jun 21, 2024
@d80tb7 d80tb7 removed the do-not-merge Do not merge this PR label Jun 21, 2024
assert.Equal(t, expectedJobError, jobError)

// If a row is bad then we should return an error and no updates should happen
_, err = ldb.db.Exec(armadacontext.Background(), "DELETE FROM job_error")
Copy link
Contributor

@JamesMurkin JamesMurkin Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be worth checking that if you try to update it with a different value, it keeps the original value?

ctx := armadacontext.New(params.HTTPRequest.Context(), logger)
result, err := getJobErrorRepo.GetJobErrorMessage(ctx, params.GetJobErrorRequest.JobID)
if err != nil {
return operations.NewGetJobRunDebugMessageBadRequest().WithPayload(conversions.ToSwaggerError(err.Error()))
Copy link
Contributor

@JamesMurkin JamesMurkin Jun 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right? Looks like a copy paste error with the wrong operation message

@d80tb7 d80tb7 enabled auto-merge (squash) June 24, 2024 09:05
@d80tb7 d80tb7 merged commit b1ae80e into master Jun 24, 2024
32 checks passed
@d80tb7 d80tb7 deleted the f/chrisma/rejection-lookout branch June 24, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants