Skip to content

Don't return Ok if update handler isn't run #2207

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

Merged
merged 1 commit into from
Feb 20, 2020
Merged

Don't return Ok if update handler isn't run #2207

merged 1 commit into from
Feb 20, 2020

Conversation

sgrif
Copy link
Contributor

@sgrif sgrif commented Feb 20, 2020

We're losing jobs to publish crates. It's not clear why. The requests
are returning 200, which implies they are being queued correctly. There
is no log to indicate that these jobs failed, so right now I believe
that this job is erroneously returning Ok when it failed. Given GitHub
is having a partial outage, that seems to reinforce this theory.

From looking at the code, this looks like the only way we could have
returned Ok when the publish failed. I believe we got some sort of
error response (or lack of one) from GitHub that caused this callback
not to be run.

@rust-highfive
Copy link

r? @carols10cents

(rust_highfive has picked a reviewer for you, use r? to override)

@sgrif
Copy link
Contributor Author

sgrif commented Feb 20, 2020

r? @pietroalbini

@sgrif sgrif force-pushed the sg-fix-bug branch 2 times, most recently from 0e3ac0b to 481a113 Compare February 20, 2020 22:19
We're losing jobs to publish crates. It's not clear why. The requests
are returning 200, which implies they are being queued correctly. There
is no log to indicate that these jobs failed, so right now I believe
that this job is erroneously returning `Ok` when it failed. Given GitHub
is having a partial outage, that seems to reinforce this theory.

From looking at the code, this looks like the only way we could have
returned `Ok` when the publish failed. I believe we got some sort of
error response (or lack of one) from GitHub that caused this callback
not to be run.
@pietroalbini
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Feb 20, 2020

📌 Commit 09814cc has been approved by pietroalbini

@bors
Copy link
Contributor

bors commented Feb 20, 2020

⌛ Testing commit 09814cc with merge 9796321...

@bors
Copy link
Contributor

bors commented Feb 20, 2020

☀️ Test successful - checks-travis
Approved by: pietroalbini
Pushing 9796321 to master...

@bors bors merged commit 9796321 into master Feb 20, 2020
@pietroalbini pietroalbini deleted the sg-fix-bug branch February 21, 2020 13:54
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.

5 participants