Skip to content
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-with-fallback won't remember if it needs to use a fallback #1967

Closed
giovanni-guidini opened this issue Jun 19, 2024 · 1 comment
Closed
Assignees
Labels
Area: Notifications Issues with notifications bug Something isn't working Medium Medium Priority Issues (to be fixed or re-evaluated in 3 months

Comments

@giovanni-guidini
Copy link

Example of PR affected by this: getsentry/sentry-python#3135 (scroll down to the statuses)

We have 2 different types of "statuses" for GitHub: statuses and checks. see GitHub docs

We use checks only in a PR scenario, but the notifier actually has a fallback to use commit statuses if it fails to send the checks. (you can know the difference because "commit status" detail lead you to Codecov UI, but "checks" lead you to the checks tab of your PR)

In this particular example we can see that the notification task failed to get PR info for this PR, but proceeded to notify later. The notifications succeeded and we have a "commit status" sent.

Later on we notify again in another task, and this time we get the PR info. This causes us to send a "check", but also means the previously sent "commit status" is not updated.

This is confusing as it looks like there are repeated statuses with different numbers.
So we need to either remember that a status exists and use statuses OR delete the status before sending checks.

@giovanni-guidini giovanni-guidini added the bug Something isn't working label Jun 19, 2024
@rohan-at-sentry rohan-at-sentry added Medium Medium Priority Issues (to be fixed or re-evaluated in 3 months Area: Notifications Issues with notifications labels Aug 20, 2024
@adrian-codecov
Copy link

adrian-codecov commented Sep 9, 2024

After doing some investigation, these are my findings:

Some definitions w/ the help of this

  • PR checks: describe the current state of a given job or task running in CI or elsewhere on a specific PR commit - they can be created by commit statuses or check runs. Both statuses and checks are associated with a specific commit. All of these below are PR checks. Image

  • Github statuses: a simple, all-purpose API for reporting PR Checks for a given commit. You can create statuses as a user with an access token and without a github app (although works with one). This came before Github checks, so it's limited in some of its capabilities

  • Github checks: latest and better API that can do everything commit statuses do + more. While more powerful, it can only be created by an authenticated GH App (for write access)

  • Useful infographics (not related to codecov) Image Image Image

  • We should try to and favour and create checks whenever possible. The requirements for creating a check are:

    • The user needs to have a github app - we need write permissions to create a check, checks:write
    • You could also create checks via the github action, if our customers are using that -> a potential add is to add these checks to a the codecov github action
  • Unfortunately, we cannot delete a previous PR check, either a commit status or a check. That makes it a bit difficult to choose one or the other

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Notifications Issues with notifications bug Something isn't working Medium Medium Priority Issues (to be fixed or re-evaluated in 3 months
Projects
None yet
Development

No branches or pull requests

3 participants