-
Notifications
You must be signed in to change notification settings - Fork 183
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
Arc 1444 keep probot running on stop #1661
Conversation
@@ -24,8 +23,6 @@ export async function stop() { | |||
return; | |||
} | |||
logger.info("Micros Lifecycle: Stopping queue processing"); | |||
// TODO: change this to `probot.close()` once we update probot to latest version | |||
probot.httpServer?.close(); |
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.
Probot in worker? wtf
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.
Nice this works wonderfully with my 5 years ago PR probot-sweep-090922
Haha didn't think of that, but yeah ! Actually me (ghe) is planning to review and merge your pr to kill probot, once we verify the new handler can process cloud web hook. |
What's in this PR?
Stop the
stop the probot httpServer
Why
For each deployment, we don't want the existing stack to stop the probot (WORKER) http server from receiving healthcheck request, in case we want to rollback, we still need the existing stack (healthcheck endpoint) to function.
Added feature flags
N/A
Affected issues
ARC-1444
How has this been tested?
ddev
Multiple deploy to make sure it still works
rollback and make sure old stack works
However, before this change, I can't make old stack (health endpoint) cease functioning during rollback to verify my theory. I suspect it is due to healthcheck in worker group is quite special, the stack may still work even that healthcheck endpoint is broken. Too much micros assumption here, can't 100% prove anything. But my gut feeling tells me we should keep the server running.
Whats Next?
N/A