Skip to content

fix(github): Revert retry on 404s#199

Merged
BYK merged 2 commits into
masterfrom
byk/fix/github-target-tag
Apr 9, 2021
Merged

fix(github): Revert retry on 404s#199
BYK merged 2 commits into
masterfrom
byk/fix/github-target-tag

Conversation

@BYK

@BYK BYK commented Apr 9, 2021

Copy link
Copy Markdown
Member

This PR reverts #177 as not only it did not address the issue it
was intending to fix, it introduced delays in where we actually
expect a 404 response. It also caused a weird failure where we
threw an error due to a 404 response which should have been ignored:
https://github.com/getsentry/publish/runs/2281793805?check_suite_focus=true#step:8:59

The retry issue was resolved via #190.

This PR reverts #177 as not only it did not address the issue it
was intending to fix, it introduced delays in where we actually
expect a 404 response. It also caused a weird failure where we
threw an error due to a 404 response which should have been ignored:
https://github.com/getsentry/publish/runs/2281793805?check_suite_focus=true#step:8:59

The retry issue was resolved via #190.
@BYK
BYK requested review from chadwhitacre, jan-auer and tonyo April 9, 2021 10:00

@chadwhitacre chadwhitacre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changelog?

@BYK
BYK requested a review from chadwhitacre April 9, 2021 12:25
@BYK
BYK merged commit da2ae96 into master Apr 9, 2021
@BYK
BYK deleted the byk/fix/github-target-tag branch April 9, 2021 18:27
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.

3 participants