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

Improve required checks creation in CI #458

Merged
merged 2 commits into from
Oct 16, 2023

Conversation

jgiannuzzi
Copy link
Member

@jgiannuzzi jgiannuzzi commented Oct 16, 2023

This PR improves how we create the required check "All required checks succeeded" in the following ways:

  • We now use a GitHub app to create the check instead of using the token provided by GitHub Actions.
    This allows us to create the check run with its own check suite instead of attaching it to the first GHA check suite, which changes when a pull request is closed and reopened, or when a new run attempt is made. See https://github.com/orgs/community/discussions/24616#discussioncomment-6088422 for more details on this very annoying GitHub limitation.
  • A check is created and set as queued or in_progress when the corresponding workflow run's status changes.
    This is needed so that the check is reset when a new run attempt is made or a PR is reopened.
  • Logging is improved.
  • Start/Completion times are added.

The following tasks need to be completed before merging this PR:

  • create and install a new GitHub app with the check:write permissions
  • add its credentials to a new create-check environment that can only be accessed by the main branch

The following tasks need to be completed after merging this PR:

  • update branch protection (or ruleset, if Change CI release logic #452 has been merged in the meantime) to enforce the required check to originate from the new GitHub app

Copy link
Contributor

@suprjinx suprjinx left a comment

Choose a reason for hiding this comment

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

I wish Github CI did not require so much custom code -- seems like never ending maintenance if Armada Project is any guide.

@jgiannuzzi
Copy link
Member Author

I wish Github CI did not require so much custom code -- seems like never ending maintenance if Armada Project is any guide.

After 4 years of using GitHub Actions, I wish so too 😞
However, it does keep improving, and we do have a particularly stringent security posture, so 🤷

@jgiannuzzi jgiannuzzi merged commit 039eeac into G-Research:main Oct 16, 2023
18 checks passed
@jgiannuzzi jgiannuzzi deleted the required-checks branch October 16, 2023 20:32
jgiannuzzi added a commit to jgiannuzzi/fasttrackml that referenced this pull request Oct 16, 2023
This PR improves how we create the required check "All required checks
succeeded" in the following ways:
* We now use a GitHub app to create the check instead of using the token
provided by GitHub Actions.
This allows us to create the check run with its own check suite instead
of attaching it to the first GHA check suite, which changes when a pull
request is closed and reopened, or when a new run attempt is made. See
https://github.com/orgs/community/discussions/24616#discussioncomment-6088422
for more details on this very annoying GitHub limitation.
* A check is created and set as `queued` or `in_progress` when the
corresponding workflow run's status changes.
This is needed so that the check is reset when a new run attempt is made
or a PR is reopened.
* Logging is improved.
* Start/Completion times are added.
suprjinx pushed a commit to suprjinx/fasttrackml that referenced this pull request Oct 23, 2023
This PR improves how we create the required check "All required checks
succeeded" in the following ways:
* We now use a GitHub app to create the check instead of using the token
provided by GitHub Actions.
This allows us to create the check run with its own check suite instead
of attaching it to the first GHA check suite, which changes when a pull
request is closed and reopened, or when a new run attempt is made. See
https://github.com/orgs/community/discussions/24616#discussioncomment-6088422
for more details on this very annoying GitHub limitation.
* A check is created and set as `queued` or `in_progress` when the
corresponding workflow run's status changes.
This is needed so that the check is reset when a new run attempt is made
or a PR is reopened.
* Logging is improved.
* Start/Completion times are added.
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.

2 participants