Skip to content

Commit

Permalink
move create_or_update_comment for PR comments into CommitContextInteg…
Browse files Browse the repository at this point in the history
…ration
  • Loading branch information
cathteng committed Aug 29, 2024
1 parent fa2fcd9 commit 9e1b16d
Show file tree
Hide file tree
Showing 11 changed files with 118 additions and 88 deletions.
4 changes: 3 additions & 1 deletion src/sentry/integrations/github/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,9 @@ def create_comment(self, repo: str, issue_id: str, data: Mapping[str, Any]) -> A
endpoint = f"/repos/{repo}/issues/{issue_id}/comments"
return self.post(endpoint, data=data)

def update_comment(self, repo: str, comment_id: str, data: Mapping[str, Any]) -> Any:
def update_comment(
self, repo: str, issue_id: str, comment_id: str, data: Mapping[str, Any]
) -> Any:
endpoint = f"/repos/{repo}/issues/comments/{comment_id}"
return self.patch(endpoint, data=data)

Expand Down
6 changes: 3 additions & 3 deletions src/sentry/integrations/github/tasks/open_pr_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@
GithubAPIErrorType,
PullRequestFile,
PullRequestIssue,
create_or_update_comment,
)
from sentry.integrations.models.repository_project_path_config import RepositoryProjectPathConfig
from sentry.integrations.services.integration import integration_service
from sentry.integrations.source_code_management.commit_context import CommitContextIntegration
from sentry.models.group import Group, GroupStatus
from sentry.models.organization import Organization
from sentry.models.project import Project
Expand Down Expand Up @@ -446,6 +446,7 @@ def open_pr_comment_workflow(pr_id: int) -> None:
return

installation = integration.get_installation(organization_id=org_id)
assert isinstance(installation, CommitContextIntegration)

client = installation.get_client()

Expand Down Expand Up @@ -595,8 +596,7 @@ def open_pr_comment_workflow(pr_id: int) -> None:
language = languages[0] if len(languages) else "not found"

try:
create_or_update_comment(
client=client,
installation.create_or_update_comment(
repo=repo,
pr_key=pull_request.key,
comment_body=comment_body,
Expand Down
11 changes: 4 additions & 7 deletions src/sentry/integrations/github/tasks/pr_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
from snuba_sdk import Request as SnubaRequest

from sentry.integrations.github.constants import ISSUE_LOCKED_ERROR_MESSAGE, RATE_LIMITED_MESSAGE
from sentry.integrations.github.tasks.utils import PullRequestIssue, create_or_update_comment
from sentry.integrations.github.tasks.utils import PullRequestIssue
from sentry.integrations.services.integration import integration_service
from sentry.integrations.source_code_management.commit_context import CommitContextIntegration
from sentry.models.group import Group
from sentry.models.groupowner import GroupOwnerType
from sentry.models.options.organization_option import OrganizationOption
Expand Down Expand Up @@ -194,19 +195,15 @@ def github_comment_workflow(pullrequest_id: int, project_id: int):
return

installation = integration.get_installation(organization_id=org_id)

# GitHubApiClient
# TODO(cathy): create helper function to fetch client for repo
client = installation.get_client()
assert isinstance(installation, CommitContextIntegration)

comment_body = format_comment(issue_comment_contents)
logger.info("github.pr_comment.comment_body", extra={"body": comment_body})

top_24_issues = issue_list[:24] # 24 is the P99 for issues-per-PR

try:
create_or_update_comment(
client=client,
installation.create_or_update_comment(
repo=repo,
pr_key=pr_key,
comment_body=comment_body,
Expand Down
67 changes: 0 additions & 67 deletions src/sentry/integrations/github/tasks/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,6 @@
from dataclasses import dataclass
from enum import Enum

from django.utils import timezone

from sentry import analytics
from sentry.integrations.github.client import GitHubApiClient
from sentry.models.pullrequest import CommentType, PullRequestComment
from sentry.models.repository import Repository
from sentry.utils import metrics

logger = logging.getLogger(__name__)


Expand All @@ -33,62 +25,3 @@ class GithubAPIErrorType(Enum):
RATE_LIMITED = "gh_rate_limited"
MISSING_PULL_REQUEST = "missing_gh_pull_request"
UNKNOWN = "unknown_api_error"


def create_or_update_comment(
client: GitHubApiClient,
repo: Repository,
pr_key: str,
comment_body: str,
pullrequest_id: int,
issue_list: list[int],
metrics_base: str,
comment_type: int = CommentType.MERGED_PR,
language: str | None = "not found",
):
pr_comment_query = PullRequestComment.objects.filter(
pull_request__id=pullrequest_id, comment_type=comment_type
)
pr_comment = pr_comment_query[0] if pr_comment_query.exists() else None

# client will raise ApiError if the request is not successful
if pr_comment is None:
resp = client.create_comment(
repo=repo.name, issue_id=str(pr_key), data={"body": comment_body}
)

current_time = timezone.now()
comment = PullRequestComment.objects.create(
external_id=resp.body["id"],
pull_request_id=pullrequest_id,
created_at=current_time,
updated_at=current_time,
group_ids=issue_list,
comment_type=comment_type,
)
metrics.incr(metrics_base.format(key="comment_created"))

if comment_type == CommentType.OPEN_PR:
analytics.record(
"open_pr_comment.created",
comment_id=comment.id,
org_id=repo.organization_id,
pr_id=pullrequest_id,
language=language,
)
else:
resp = client.update_comment(
repo=repo.name, comment_id=pr_comment.external_id, data={"body": comment_body}
)
metrics.incr(metrics_base.format(key="comment_updated"))
pr_comment.updated_at = timezone.now()
pr_comment.group_ids = issue_list
pr_comment.save()

# TODO(cathy): Figure out a way to track average rate limit left for GH client

logger_event = metrics_base.format(key="create_or_update_comment")
logger.info(
logger_event,
extra={"new_comment": pr_comment is None, "pr_key": pr_key, "repo": repo.name},
)
16 changes: 14 additions & 2 deletions src/sentry/integrations/gitlab/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,25 @@ def create_issue(self, project, data):
"""
return self.post(GitLabApiClientPath.issues.format(project=project), data=data)

def create_issue_comment(self, project_id, issue_id, data):
def create_comment(self, repo: str, issue_id: str, data: dict[str, Any]):
"""Create an issue note/comment
See https://docs.gitlab.com/ee/api/notes.html#create-new-issue-note
"""
return self.post(
GitLabApiClientPath.notes.format(project=project_id, issue_id=issue_id), data=data
GitLabApiClientPath.create_note.format(project=repo, issue_id=issue_id), data=data
)

def update_comment(self, repo: str, issue_id: str, comment_id: str, data: dict[str, Any]):
"""Modify existing issue note
See https://docs.gitlab.com/ee/api/notes.html#modify-existing-issue-note
"""
return self.put(
GitLabApiClientPath.update_note.format(
project=repo, issue_id=issue_id, note_id=comment_id
),
data=data,
)

def search_project_issues(self, project_id, query, iids=None):
Expand Down
5 changes: 2 additions & 3 deletions src/sentry/integrations/gitlab/issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,8 @@ def after_link_issue(self, external_issue, **kwargs):
return

try:
client.create_issue_comment(
project_id=project_id, issue_id=issue_id, data={"body": comment}
)
# GitLab has projects which are equivalent to repos
client.create_comment(repo=project_id, issue_id=issue_id, data={"body": comment})
except ApiError as e:
raise IntegrationError(self.message_from_error(e))

Expand Down
3 changes: 2 additions & 1 deletion src/sentry/integrations/gitlab/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ class GitLabApiClientPath:
hooks = "/hooks"
issue = "/projects/{project}/issues/{issue}"
issues = "/projects/{project}/issues"
notes = "/projects/{project}/issues/{issue_id}/notes"
create_note = "/projects/{project}/issues/{issue_id}/notes"
update_note = "/projects/{project}/issues/{issue_id}/notes/{note_id}"
project = "/projects/{project}"
project_issues = "/projects/{project}/issues"
project_hooks = "/projects/{project}/hooks"
Expand Down
83 changes: 83 additions & 0 deletions src/sentry/integrations/source_code_management/commit_context.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
from __future__ import annotations

import logging
from abc import ABC, abstractmethod
from collections.abc import Mapping, Sequence
from dataclasses import dataclass
from datetime import datetime
from typing import Any

from django.utils import timezone

from sentry import analytics
from sentry.auth.exceptions import IdentityNotValid
from sentry.integrations.models.repository_project_path_config import RepositoryProjectPathConfig
from sentry.models.pullrequest import CommentType, PullRequestComment
from sentry.models.repository import Repository
from sentry.users.models.identity import Identity
from sentry.utils import metrics

logger = logging.getLogger(__name__)


@dataclass
Expand All @@ -36,6 +44,10 @@ class FileBlameInfo(SourceLineInfo):


class CommitContextIntegration(ABC):
"""
Base class for integrations that include commit context features: suspect commits, suspect PR comments
"""

@abstractmethod
def get_client(self) -> CommitContextClient:
raise NotImplementedError
Expand Down Expand Up @@ -67,6 +79,67 @@ def get_commit_context_all_frames(
"""
return self.get_blame_for_files(files, extra)

def create_or_update_comment(
self,
repo: Repository,
pr_key: str,
comment_body: str,
pullrequest_id: int,
issue_list: list[int],
metrics_base: str,
comment_type: int = CommentType.MERGED_PR,
language: str | None = "not found",
):
client = self.get_client()

pr_comment_query = PullRequestComment.objects.filter(
pull_request__id=pullrequest_id, comment_type=comment_type
)
pr_comment = pr_comment_query[0] if pr_comment_query.exists() else None

# client will raise ApiError if the request is not successful
if pr_comment is None:
resp = client.create_comment(
repo=repo.name, issue_id=str(pr_key), data={"body": comment_body}
)

current_time = timezone.now()
comment = PullRequestComment.objects.create(
external_id=resp.body["id"],
pull_request_id=pullrequest_id,
created_at=current_time,
updated_at=current_time,
group_ids=issue_list,
comment_type=comment_type,
)
metrics.incr(metrics_base.format(key="comment_created"))

if comment_type == CommentType.OPEN_PR:
analytics.record(
"open_pr_comment.created",
comment_id=comment.id,
org_id=repo.organization_id,
pr_id=pullrequest_id,
language=language,
)
else:
resp = client.update_comment(
repo=repo.name,
issue_id=str(pr_key),
comment_id=pr_comment.external_id,
data={"body": comment_body},
)
metrics.incr(metrics_base.format(key="comment_updated"))
pr_comment.updated_at = timezone.now()
pr_comment.group_ids = issue_list
pr_comment.save()

logger_event = metrics_base.format(key="create_or_update_comment")
logger.info(
logger_event,
extra={"new_comment": pr_comment is None, "pr_key": pr_key, "repo": repo.name},
)


class CommitContextClient(ABC):
@abstractmethod
Expand All @@ -75,3 +148,13 @@ def get_blame_for_files(
) -> list[FileBlameInfo]:
"""Get the blame for a list of files. This method should include custom metrics for the specific integration implementation."""
raise NotImplementedError

@abstractmethod
def create_comment(self, repo: str, issue_id: str, data: Mapping[str, Any]) -> Any:
raise NotImplementedError

@abstractmethod
def update_comment(
self, repo: str, issue_id: str, comment_id: str, data: Mapping[str, Any]
) -> Any:
raise NotImplementedError
1 change: 1 addition & 0 deletions src/sentry/tasks/commit_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
PR_COMMENT_TASK_TTL = timedelta(minutes=5).total_seconds()
PR_COMMENT_WINDOW = 14 # days

# TODO: replace this with isinstance(installation, CommitContextIntegration)
PR_COMMENT_SUPPORTED_PROVIDERS = {"integrations:github"}

logger = logging.getLogger(__name__)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,9 @@ class TestGetFilenames(GithubCommentTestCase):
def setUp(self):
super().setUp()
self.pr = self.create_pr_issues()
self.mock_metrics = patch("sentry.integrations.github.tasks.pr_comment.metrics").start()
self.mock_metrics = patch(
"sentry.integrations.source_code_management.commit_context.metrics"
).start()
self.gh_path = self.base_url + "/repos/getsentry/sentry/pulls/{pull_number}/files"
installation = self.integration.get_installation(organization_id=self.organization.id)
self.gh_client = installation.get_client()
Expand Down Expand Up @@ -823,7 +825,7 @@ def test_comment_format_missing_language(self):
)
@patch("sentry.integrations.github.tasks.open_pr_comment.get_top_5_issues_by_count_for_file")
@patch("sentry.integrations.github.tasks.open_pr_comment.safe_for_comment")
@patch("sentry.integrations.github.tasks.utils.metrics")
@patch("sentry.integrations.source_code_management.commit_context.metrics")
class TestOpenPRCommentWorkflow(IntegrationTestCase, CreateEventTestCase):
base_url = "https://api.github.com"

Expand Down
4 changes: 2 additions & 2 deletions tests/sentry/integrations/github/tasks/test_pr_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ def setUp(self):
self.cache_key = DEBOUNCE_PR_COMMENT_CACHE_KEY(self.pr.id)

@patch("sentry.integrations.github.tasks.pr_comment.get_top_5_issues_by_count")
@patch("sentry.integrations.github.tasks.utils.metrics")
@patch("sentry.integrations.source_code_management.commit_context.metrics")
@responses.activate
def test_comment_workflow(self, mock_metrics, mock_issues):
group_objs = Group.objects.order_by("id").all()
Expand Down Expand Up @@ -439,7 +439,7 @@ def test_comment_workflow(self, mock_metrics, mock_issues):
mock_metrics.incr.assert_called_with("github_pr_comment.comment_created")

@patch("sentry.integrations.github.tasks.pr_comment.get_top_5_issues_by_count")
@patch("sentry.integrations.github.tasks.utils.metrics")
@patch("sentry.integrations.source_code_management.commit_context.metrics")
@responses.activate
@freeze_time(datetime(2023, 6, 8, 0, 0, 0, tzinfo=UTC))
def test_comment_workflow_updates_comment(self, mock_metrics, mock_issues):
Expand Down

0 comments on commit 9e1b16d

Please sign in to comment.