-
Notifications
You must be signed in to change notification settings - Fork 25
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
check if health check already exist in the queue #156
base: main
Are you sure you want to change the base?
Conversation
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.
:)
health.go
Outdated
@@ -429,6 +429,51 @@ func deleteJunkOpsWorkflow(ctx context.Context, workflowHealth WorkflowHealth) e | |||
return nil | |||
} | |||
|
|||
func checkQueueForHealthRun(ctx context.Context) error{ | |||
baseUrl := "https://shuffler.io" | |||
if os.Getenv("SHUFFLE_CLOUDRUN_URL") != "" { |
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.
Just always check BASE_URL > CLOUDRUN_URL, as CLOUDRUN_URL is the override we use everywhere
health.go
Outdated
baseUrl = os.Getenv("SHUFFLE_CLOUDRUN_URL") | ||
} | ||
|
||
if project.Environment == "onprem" { |
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.
This may be wrong, so instead of == "onprem"
do != "cloud"
health.go
Outdated
|
||
|
||
client := GetExternalClient(baseUrl) | ||
fullUrl := fmt.Sprintf("%s/api/v1/workflows/queue", baseUrl) |
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.
Can't you read this directly from the database? Using the API seems silly lol.
@@ -565,6 +610,41 @@ func RunOpsHealthCheck(resp http.ResponseWriter, request *http.Request) { | |||
} | |||
} else { | |||
// FIXME: Add a check for if it's been <interval> length at least between runs. This is 15 minutes by default. | |||
err := checkQueueForHealthRun(ctx) | |||
if err != nil { | |||
log.Printf("[ERROR] Failed running health check (4): %s", 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.
Shouldn't this then fail out? Why are we not handling the error?
ready for round 2 of review @frikky :) |
No description provided.