Skip to content

Commit

Permalink
benchalerts: remove GitHubStatusStep (conbench#1286)
Browse files Browse the repository at this point in the history 
* benchalerts: remove GitHubStatusStep

* whoops

* typo
  • Loading branch information
Austin Dickey authored May 31, 2023
1 parent 81a0272 commit 432e53f
Show file tree
Hide file tree
Showing 11 changed files with 32 additions and 501 deletions.
7 changes: 4 additions & 3 deletions benchalerts/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,16 @@ This package is intended to make the following steps easier in CI. Before these
it is assumed that an execution environment has performed a run of benchmarks and
submitted the results to Conbench.

- Hit the Conbench API to understand if there were any:
- Hit the Conbench API, which uses the [lookback z-score
method](https://conbench.github.io/conbench/pages/lookback_zscore.html), to understand
if there were any:
- errors
- regressions (with configuration for how these regressions may be detected)
- improvements (with configuration for how these improvements may be detected)
- Format and submit a summary of these findings to various places (again, with
configuration):
- GitHub Status on a commit
- GitHub Check on a commit with a Markdown summary
- Comment on a GitHub pull request

In the future, there will be more places to submit alerts/reports/summaries, and more
configuration possible.
Expand Down Expand Up @@ -100,7 +102,6 @@ follow these instructions.
we don't need GitHub to push webhook events to the App.
1. For full use of this package, the App requires the following permissions:
- Repository > Checks > Read and Write
- Repository > Commit statuses > Read and Write
- Repository > Pull requests > Read and Write
1. After creating the App, save the App ID for later.
1. For the App's photo, use [this
Expand Down
56 changes: 0 additions & 56 deletions benchalerts/benchalerts/integrations/github.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import datetime
import enum
import os
import textwrap
from typing import TYPE_CHECKING, Any, Dict, Optional

import jwt
Expand Down Expand Up @@ -99,14 +98,6 @@ def get_app_access_token(self) -> str:
return token_info["token"]


# used as inputs to some GitHubRepoClient methods
class StatusState(str, enum.Enum):
ERROR = "error"
FAILURE = "failure"
PENDING = "pending"
SUCCESS = "success"


# used as inputs to some GitHubRepoClient methods
class CheckStatus(str, enum.Enum):
# statuses
Expand Down Expand Up @@ -208,53 +199,6 @@ def create_pull_request_comment(
)
return self.post(f"/issues/{pull_number}/comments", json={"body": comment})

def update_commit_status(
self,
commit_hash: str,
title: str,
description: str,
state: StatusState,
details_url: Optional[str] = None,
) -> dict:
"""Update the GitHub status of a commit.
A commit may have many statuses, each with their own title. Updating a previous
status with the same title for a given commit will result in overwriting that
status on that commit.
Parameters
----------
commit_hash
The 40-character hash of the commit to update.
title
The title of the status. Subsequent updates with the same title will update
the same status.
description
The short description of the status.
state
The overall status of the commit. Must be one of the StatusState enum
values.
details_url
A URL to be linked to when clicking on status Details. Default None.
Returns
-------
dict
GitHub's details about the new status.
"""
if not isinstance(state, StatusState):
fatal_and_log("state must be a StatusState", etype=TypeError)

json = {
"state": state.value,
"description": textwrap.shorten(description, 140),
"context": title,
}
if details_url:
json["target_url"] = details_url

return self.post(f"/statuses/{commit_hash}", json=json)

def update_check(
self,
name: str,
Expand Down
4 changes: 0 additions & 4 deletions benchalerts/benchalerts/pipeline_steps/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@
GitHubCheckErrorHandler,
GitHubCheckStep,
GitHubPRCommentAboutCheckStep,
GitHubStatusErrorHandler,
GitHubStatusStep,
)

__all__ = [
Expand All @@ -18,6 +16,4 @@
"GitHubCheckErrorHandler",
"GitHubCheckStep",
"GitHubPRCommentAboutCheckStep",
"GitHubStatusErrorHandler",
"GitHubStatusStep",
]
170 changes: 1 addition & 169 deletions benchalerts/benchalerts/pipeline_steps/github.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from ..alert_pipeline import AlertPipelineErrorHandler, AlertPipelineStep
from ..conbench_dataclasses import FullComparisonInfo
from ..integrations.github import CheckStatus, GitHubRepoClient, StatusState
from ..integrations.github import CheckStatus, GitHubRepoClient
from ..message_formatting import (
_clean,
_Pluralizer,
Expand Down Expand Up @@ -152,118 +152,6 @@ def _default_check_details(full_comparison: FullComparisonInfo) -> Optional[str]
return github_check_details(full_comparison)


class GitHubStatusStep(AlertPipelineStep):
"""An ``AlertPipeline`` step to update a GitHub Status on a commit on GitHub, based
on information collected in a previously-run ``GetConbenchZComparisonStep`` or
``GetConbenchZComparisonForRunsStep``.
Parameters
----------
commit_hash
The commit hash to update.
comparison_step_name
The name of the ``GetConbenchZComparisonStep`` or
``GetConbenchZComparisonForRunsStep`` that ran earlier in the pipeline.If
unspecified in that step, it defaults to the name of the step class, e.g.
``"GetConbenchZComparisonStep"``.
repo
The repo name to post the status to, in the form "owner/repo". Either provide
this or ``github_client``.
github_client
A GitHubRepoClient instance. Either provide this or ``repo``.
step_name
The name for this step. If not given, will default to this class's name.
Returns
-------
dict
The response body from the GitHub HTTP API as a dict.
Notes
-----
Environment variables
~~~~~~~~~~~~~~~~~~~~~
``GITHUB_APP_ID``
The ID of a GitHub App that has been set up according to this package's
instructions and installed to your repo. Recommended over ``GITHUB_API_TOKEN``.
Only required if ``repo`` is provided instead of ``github_client``.
``GITHUB_APP_PRIVATE_KEY``
The private key file contents of a GitHub App that has been set up according to
this package's instructions and installed to your repo. Recommended over
``GITHUB_API_TOKEN``. Only required if ``repo`` is provided instead of
``github_client``.
``GITHUB_API_TOKEN``
A GitHub Personal Access Token with the ``repo:status`` permission. Only
required if not authenticating with a GitHub App, and if ``repo`` is provided
instead of ``github_client``.
"""

def __init__(
self,
commit_hash: str,
comparison_step_name: str,
repo: Optional[str] = None,
github_client: Optional[GitHubRepoClient] = None,
step_name: Optional[str] = None,
) -> None:
super().__init__(step_name=step_name)
self.github_client = github_client or GitHubRepoClient(repo=repo or "")
self.commit_hash = commit_hash
self.comparison_step_name = comparison_step_name

def run_step(self, previous_outputs: Dict[str, Any]) -> dict:
full_comparison: FullComparisonInfo = previous_outputs[
self.comparison_step_name
]

res = self.github_client.update_commit_status(
commit_hash=self.commit_hash,
title="conbench",
description=self._default_status_description(full_comparison),
state=self._default_status_state(full_comparison),
details_url=full_comparison.app_url,
)
log.debug(res)
return res

# TODO: a way to override the following behavior
# (see https://github.com/conbench/conbench/issues/774)

@staticmethod
def _default_status_state(full_comparison: FullComparisonInfo) -> StatusState:
"""Return a different status based on errors and regressions."""
if full_comparison.results_with_errors:
# results with errors require action
return StatusState.FAILURE
elif full_comparison.results_with_z_regressions:
# no errors, but results with regressions are still a failure
return StatusState.FAILURE
elif full_comparison.has_any_z_analyses:
# we analyzed z-scores and didn't find errors or regressions
return StatusState.SUCCESS
elif full_comparison.has_any_contender_results:
# we have results but couldn't analyze z-scores (normal at beginning of history)
return StatusState.SUCCESS
else:
# we don't have results; this requires action
return StatusState.FAILURE

@staticmethod
def _default_status_description(full_comparison: FullComparisonInfo) -> str:
if full_comparison.results_with_errors:
return "Some benchmarks had errors"
elif not full_comparison.has_any_z_analyses:
return "Could not do the lookback z-score analysis"
else:
pluralizer = _Pluralizer(full_comparison.results_with_z_regressions)
were = pluralizer.were
s = pluralizer.s
return (
f"There {were} {len(full_comparison.results_with_z_regressions)} "
f"benchmark regression{s} in this commit"
)


class GitHubPRCommentAboutCheckStep(AlertPipelineStep):
"""An ``AlertPipeline`` step to make a comment on a PR about a GitHub Check that was
created by a previously-run ``GitHubCheckStep``. This is useful if you're running
Expand Down Expand Up @@ -403,59 +291,3 @@ def handle_error(self, exc: BaseException, traceback: str) -> None:
details_url=self.build_url,
)
log.debug(res)


class GitHubStatusErrorHandler(AlertPipelineErrorHandler):
"""Handle errors in a pipeline by updating a GitHub Status.
Parameters
----------
commit_hash
The commit hash to update.
repo
The repo name to post the status to, in the form "owner/repo". Either provide
this or ``github_client``.
github_client
A GitHubRepoClient instance. Either provide this or ``repo``.
build_url
An optional build URL to include in the GitHub Check.
Notes
-----
Environment variables
~~~~~~~~~~~~~~~~~~~~~
``GITHUB_APP_ID``
The ID of a GitHub App that has been set up according to this package's
instructions and installed to your repo. Recommended over ``GITHUB_API_TOKEN``.
Only required if ``repo`` is provided instead of ``github_client``.
``GITHUB_APP_PRIVATE_KEY``
The private key file contents of a GitHub App that has been set up according to
this package's instructions and installed to your repo. Recommended over
``GITHUB_API_TOKEN``. Only required if ``repo`` is provided instead of
``github_client``.
``GITHUB_API_TOKEN``
A GitHub Personal Access Token with the ``repo:status`` permission. Only
required if not authenticating with a GitHub App, and if ``repo`` is provided
instead of ``github_client``.
"""

def __init__(
self,
commit_hash: str,
repo: Optional[str] = None,
github_client: Optional[GitHubRepoClient] = None,
build_url: Optional[str] = None,
) -> None:
self.commit_hash = commit_hash
self.github_client = github_client or GitHubRepoClient(repo=repo or "")
self.build_url = build_url

def handle_error(self, exc: BaseException, traceback: str) -> None:
res = self.github_client.update_commit_status(
commit_hash=self.commit_hash,
title="conbench",
description=f"Failed finding regressions: {exc}",
state=StatusState.ERROR,
details_url=self.build_url,
)
log.debug(res)
Loading

0 comments on commit 432e53f

Please sign in to comment.