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

Feature flagging via the GitHub API #856

Merged
merged 6 commits into from
Dec 16, 2021
Merged

Conversation

henrymercer
Copy link
Contributor

@henrymercer henrymercer commented Dec 14, 2021

This PR introduces feature flagging to the CodeQL Action via a GitHub API endpoint /repositories/:repository_id/code-scanning/codeql-action/features (see backlinked issue). This API endpoint only exists on Dotcom, so for GHES and GHAE feature flags will all be marked as disabled.

This is a general purpose solution that'll allow us to add many feature flags to the CodeQL Action while only calling a single API endpoint. In this PR we use this mechanism to replace the database uploading feature flags; in the future we'll use it to decide whether to run ML-powered queries.

I've written some unit tests for the feature flagging class, and verified that database upload works as expected on a repo with the feature flag enabled. Ideas for further testing welcome.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

@henrymercer henrymercer marked this pull request as ready for review December 14, 2021 21:18
@henrymercer henrymercer requested a review from a team as a code owner December 14, 2021 21:18
Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

I haven't reviewed in huge detail. I can do that tomorrow or leave it to the CodeQL team. I have a couple of initial comments though.

src/feature-flags.ts Outdated Show resolved Hide resolved
src/feature-flags.ts Outdated Show resolved Hide resolved
As suggested in review: The `GITHUB_REPOSITORY` environment variable is
only available on Actions. Passing it in explicitly avoids potentially
crashing if this code is called from the runner.
Copy link
Contributor

@aeisenberg aeisenberg left a comment

Choose a reason for hiding this comment

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

Looks good. A few minor comments.

src/analyze-action-env.test.ts Outdated Show resolved Hide resolved
src/feature-flags.ts Outdated Show resolved Hide resolved
src/feature-flags.ts Show resolved Hide resolved
Copy link
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

LGTM too once @aeisenberg is happy

src/database-upload.test.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@aeisenberg aeisenberg left a comment

Choose a reason for hiding this comment

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

Nice.

@henrymercer henrymercer merged commit 249c7ff into main Dec 16, 2021
@henrymercer henrymercer deleted the henrymercer/feature-flagging branch December 16, 2021 16:18
@github-actions github-actions bot mentioned this pull request Jan 11, 2022
5 tasks
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.

None yet

3 participants