Add client disconnect check to build handler loop #10295
Conversation
jwhonce
changed the title
| @@ -553,6 +553,10 @@ loop: | |||
| } | |||
| } | |||
| break loop | |||
| case <-r.Context().Done(): | |||
| cancel() | |||
| logrus.Infof("Client disconnect reported for build %q / %q.", registry, query.Dockerfile) | |||
There was a problem hiding this comment.
Just curious, but my expectation would be to see this as at least a Warn?
There was a problem hiding this comment.
@edsantiago My thinking is from the service POV, a client disconnecting is not a error condition simply a state change.
|
LGTM |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, jwhonce The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
[NO TESTS NEEDED] In process of debugging added request channel check and logging message to build loop. Unable to recreate build drop issue after this. 68k build iterations without fail. Signed-off-by: Jhon Honce <[email protected]>
|
LGTM |
|
/lgtm |
|
/hold CI isn't finished yet |
openshift-ci
bot
added
the
do-not-merge/hold
|
@edsantiago I thought if you did a |
|
I don't know. I was told years ago that slash-lgtm did really really bad things if CI wasn't finished. Something about "cranking" or "churning" or something. Perhaps that bug has been fixed and I just wasn't aware of it. @cevich any idea? |
edsantiago
removed
the
do-not-merge/hold
|
AFAIK it is not fixed. CI will spin trying to merge and block all other
merges until CI completes.
…On Mon, May 10, 2021 at 13:58 OpenShift Merge Robot < ***@***.***> wrote:
Merged #10295 <#10295> into
master.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#10295 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3AOCAYI4APT6W6VGYAT7DTNANCTANCNFSM44RTXUKQ>
.
|
|
@jwhonce, so sorry, it is not fixed. This is a run based on master @ 76c8577, which includes this PR. Unfortunately, there's no way (yet?) to capture the server logs. Should I look into a way to preserve those? |
Ya, it's safer to assume it's still broken. The bot's configuration is super-duper complex, and shared with the OpenShift team. I'd rather not rock the boat (or even even make it lean slightly). |
github-actions
bot
added
the
locked - please file new issue/PR
In process of debugging #10154, I added request channel check and logging message to build loop. Unable to recreate build drop issue after this. 68k build iterations without fail.
RE: #10154
Signed-off-by: Jhon Honce [email protected]