Skip to content

Commit bae6fa4

Browse files
jianyuanjianyuan
committed
ref: move safe_for_comment and get_pr_files
1 parent 136d53c commit bae6fa4

File tree

9 files changed

+178
-114
lines changed

9 files changed

+178
-114
lines changed

‎src/sentry/integrations/github/client.py

+3-2
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
from sentry.integrations.source_code_management.repo_trees import RepoTreesClient
2727
from sentry.integrations.source_code_management.repository import RepositoryClient
2828
from sentry.integrations.types import EXTERNAL_PROVIDERS, ExternalProviders
29+
from sentry.models.pullrequest import PullRequest
2930
from sentry.models.repository import Repository
3031
from sentry.shared_integrations.client.proxy import IntegrationProxyClient
3132
from sentry.shared_integrations.exceptions import ApiError, ApiRateLimitedError
@@ -254,13 +255,13 @@ def get_pullrequest_from_commit(self, repo: str, sha: str) -> Any:
254255
"""
255256
return self.get(f"/repos/{repo}/commits/{sha}/pulls")
256257

257-
def get_pullrequest_files(self, repo: str, pull_number: str) -> Any:
258+
def get_pullrequest_files(self, repo: Repository, pr: PullRequest) -> Any:
258259
"""
259260
https://docs.github.com/en/rest/pulls/pulls#list-pull-requests-files
260261
261262
Returns up to 30 files associated with a pull request. Responses are paginated.
262263
"""
263-
return self.get(f"/repos/{repo}/pulls/{pull_number}/files")
264+
return self.get(f"/repos/{repo.name}/pulls/{pr.key}/files")
264265

265266
def get_repo(self, repo: str) -> Any:
266267
"""

‎src/sentry/integrations/github/integration.py

+83
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,22 @@
2828
)
2929
from sentry.integrations.github.constants import ISSUE_LOCKED_ERROR_MESSAGE, RATE_LIMITED_MESSAGE
3030
from sentry.integrations.github.tasks.link_all_repos import link_all_repos
31+
from sentry.integrations.github.tasks.utils import GithubAPIErrorType
3132
from sentry.integrations.models.integration import Integration
3233
from sentry.integrations.models.organization_integration import OrganizationIntegration
3334
from sentry.integrations.services.repository import RpcRepository, repository_service
3435
from sentry.integrations.source_code_management.commit_context import (
36+
OPEN_PR_MAX_FILES_CHANGED,
37+
OPEN_PR_MAX_LINES_CHANGED,
38+
OPEN_PR_METRICS_BASE,
3539
CommitContextIntegration,
3640
CommitContextOrganizationOptionKeys,
3741
CommitContextReferrerIds,
3842
CommitContextReferrers,
43+
PullRequestFile,
3944
PullRequestIssue,
4045
)
46+
from sentry.integrations.source_code_management.language_parsers import PATCH_PARSERS
4147
from sentry.integrations.source_code_management.repo_trees import RepoTreesIntegration
4248
from sentry.integrations.source_code_management.repository import RepositoryIntegration
4349
from sentry.integrations.tasks.migrate_repo import migrate_repo
@@ -47,6 +53,7 @@
4753
)
4854
from sentry.models.group import Group
4955
from sentry.models.organization import Organization
56+
from sentry.models.pullrequest import PullRequest
5057
from sentry.models.repository import Repository
5158
from sentry.organizations.absolute_url import generate_organization_url
5259
from sentry.organizations.services.organization.model import RpcOrganization
@@ -391,6 +398,82 @@ def on_create_or_update_comment_error(self, api_error: ApiError, metrics_base: s
391398

392399
return False
393400

401+
def get_pr_files_safe_for_comment(
402+
self, repo: Repository, pr: PullRequest
403+
) -> list[dict[str, str]]:
404+
client = self.get_client()
405+
406+
logger.info("github.open_pr_comment.check_safe_for_comment")
407+
try:
408+
pr_files = client.get_pullrequest_files(repo=repo, pr=pr)
409+
except ApiError as e:
410+
logger.info("github.open_pr_comment.api_error")
411+
if e.json and RATE_LIMITED_MESSAGE in e.json.get("message", ""):
412+
metrics.incr(
413+
OPEN_PR_METRICS_BASE.format(integration="github", key="api_error"),
414+
tags={"type": GithubAPIErrorType.RATE_LIMITED.value, "code": e.code},
415+
)
416+
elif e.code == 404:
417+
metrics.incr(
418+
OPEN_PR_METRICS_BASE.format(integration="github", key="api_error"),
419+
tags={"type": GithubAPIErrorType.MISSING_PULL_REQUEST.value, "code": e.code},
420+
)
421+
else:
422+
metrics.incr(
423+
OPEN_PR_METRICS_BASE.format(integration="github", key="api_error"),
424+
tags={"type": GithubAPIErrorType.UNKNOWN.value, "code": e.code},
425+
)
426+
logger.exception(
427+
"github.open_pr_comment.unknown_api_error", extra={"error": str(e)}
428+
)
429+
return []
430+
431+
changed_file_count = 0
432+
changed_lines_count = 0
433+
filtered_pr_files = []
434+
435+
patch_parsers = PATCH_PARSERS
436+
# NOTE: if we are testing beta patch parsers, add check here
437+
438+
for file in pr_files:
439+
filename = file["filename"]
440+
# we only count the file if it's modified and if the file extension is in the list of supported file extensions
441+
# we cannot look at deleted or newly added files because we cannot extract functions from the diffs
442+
if file["status"] != "modified" or filename.split(".")[-1] not in patch_parsers:
443+
continue
444+
445+
changed_file_count += 1
446+
changed_lines_count += file["changes"]
447+
filtered_pr_files.append(file)
448+
449+
if changed_file_count > OPEN_PR_MAX_FILES_CHANGED:
450+
metrics.incr(
451+
OPEN_PR_METRICS_BASE.format(integration="github", key="rejected_comment"),
452+
tags={"reason": "too_many_files"},
453+
)
454+
return []
455+
if changed_lines_count > OPEN_PR_MAX_LINES_CHANGED:
456+
metrics.incr(
457+
OPEN_PR_METRICS_BASE.format(integration="github", key="rejected_comment"),
458+
tags={"reason": "too_many_lines"},
459+
)
460+
return []
461+
462+
return filtered_pr_files
463+
464+
def get_pr_files(self, pr_files: list[dict[str, str]]) -> list[PullRequestFile]:
465+
# new files will not have sentry issues associated with them
466+
# only fetch Python files
467+
pullrequest_files = [
468+
PullRequestFile(filename=file["filename"], patch=file["patch"])
469+
for file in pr_files
470+
if "patch" in file
471+
]
472+
473+
logger.info("github.open_pr_comment.pr_filenames", extra={"count": len(pullrequest_files)})
474+
475+
return pullrequest_files
476+
394477
def format_open_pr_comment(self, issue_tables: list[str]) -> str:
395478
comment_body_template = """\
396479
## 🔍 Existing Issues For Review

‎src/sentry/integrations/github/tasks/open_pr_comment.py

+2-87
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,10 @@
55
from typing import Any
66

77
from sentry.constants import EXTENSION_LANGUAGE_MAP, ObjectStatus
8-
from sentry.integrations.github.client import GitHubApiClient
9-
from sentry.integrations.github.constants import RATE_LIMITED_MESSAGE
10-
from sentry.integrations.github.tasks.utils import GithubAPIErrorType
118
from sentry.integrations.services.integration import integration_service
129
from sentry.integrations.source_code_management.commit_context import (
13-
OPEN_PR_MAX_FILES_CHANGED,
14-
OPEN_PR_MAX_LINES_CHANGED,
1510
OPEN_PR_METRICS_BASE,
1611
CommitContextIntegration,
17-
PullRequestFile,
1812
)
1913
from sentry.integrations.source_code_management.language_parsers import PATCH_PARSERS
2014
from sentry.models.organization import Organization
@@ -30,83 +24,6 @@
3024
logger = logging.getLogger(__name__)
3125

3226

33-
# TODO(cathy): Change the client typing to allow for multiple SCM Integrations
34-
def safe_for_comment(
35-
gh_client: GitHubApiClient, repository: Repository, pull_request: PullRequest
36-
) -> list[dict[str, str]]:
37-
logger.info("github.open_pr_comment.check_safe_for_comment")
38-
try:
39-
pr_files = gh_client.get_pullrequest_files(
40-
repo=repository.name, pull_number=pull_request.key
41-
)
42-
except ApiError as e:
43-
logger.info("github.open_pr_comment.api_error")
44-
if e.json and RATE_LIMITED_MESSAGE in e.json.get("message", ""):
45-
metrics.incr(
46-
OPEN_PR_METRICS_BASE.format(integration="github", key="api_error"),
47-
tags={"type": GithubAPIErrorType.RATE_LIMITED.value, "code": e.code},
48-
)
49-
elif e.code == 404:
50-
metrics.incr(
51-
OPEN_PR_METRICS_BASE.format(integration="github", key="api_error"),
52-
tags={"type": GithubAPIErrorType.MISSING_PULL_REQUEST.value, "code": e.code},
53-
)
54-
else:
55-
metrics.incr(
56-
OPEN_PR_METRICS_BASE.format(integration="github", key="api_error"),
57-
tags={"type": GithubAPIErrorType.UNKNOWN.value, "code": e.code},
58-
)
59-
logger.exception("github.open_pr_comment.unknown_api_error", extra={"error": str(e)})
60-
return []
61-
62-
changed_file_count = 0
63-
changed_lines_count = 0
64-
filtered_pr_files = []
65-
66-
patch_parsers = PATCH_PARSERS
67-
# NOTE: if we are testing beta patch parsers, add check here
68-
69-
for file in pr_files:
70-
filename = file["filename"]
71-
# we only count the file if it's modified and if the file extension is in the list of supported file extensions
72-
# we cannot look at deleted or newly added files because we cannot extract functions from the diffs
73-
if file["status"] != "modified" or filename.split(".")[-1] not in patch_parsers:
74-
continue
75-
76-
changed_file_count += 1
77-
changed_lines_count += file["changes"]
78-
filtered_pr_files.append(file)
79-
80-
if changed_file_count > OPEN_PR_MAX_FILES_CHANGED:
81-
metrics.incr(
82-
OPEN_PR_METRICS_BASE.format(integration="github", key="rejected_comment"),
83-
tags={"reason": "too_many_files"},
84-
)
85-
return []
86-
if changed_lines_count > OPEN_PR_MAX_LINES_CHANGED:
87-
metrics.incr(
88-
OPEN_PR_METRICS_BASE.format(integration="github", key="rejected_comment"),
89-
tags={"reason": "too_many_lines"},
90-
)
91-
return []
92-
93-
return filtered_pr_files
94-
95-
96-
def get_pr_files(pr_files: list[dict[str, str]]) -> list[PullRequestFile]:
97-
# new files will not have sentry issues associated with them
98-
# only fetch Python files
99-
pullrequest_files = [
100-
PullRequestFile(filename=file["filename"], patch=file["patch"])
101-
for file in pr_files
102-
if "patch" in file
103-
]
104-
105-
logger.info("github.open_pr_comment.pr_filenames", extra={"count": len(pullrequest_files)})
106-
107-
return pullrequest_files
108-
109-
11027
@instrumented_task(
11128
name="sentry.integrations.github.tasks.open_pr_comment_workflow",
11229
silo_mode=SiloMode.REGION,
@@ -173,12 +90,10 @@ def open_pr_comment_workflow(pr_id: int) -> None:
17390
installation = integration.get_installation(organization_id=organization.id)
17491
assert isinstance(installation, CommitContextIntegration)
17592

176-
client = installation.get_client()
177-
17893
# CREATING THE COMMENT
17994

18095
# fetch the files in the PR and determine if it is safe to comment
181-
pr_files = safe_for_comment(gh_client=client, repository=repo, pull_request=pr)
96+
pr_files = installation.get_pr_files_safe_for_comment(repo=repo, pr=pr)
18297

18398
if len(pr_files) == 0:
18499
logger.info(
@@ -190,7 +105,7 @@ def open_pr_comment_workflow(pr_id: int) -> None:
190105
)
191106
return
192107

193-
pullrequest_files = get_pr_files(pr_files)
108+
pullrequest_files = installation.get_pr_files(pr_files)
194109

195110
issue_table_contents = {}
196111
top_issues_per_file = []

‎src/sentry/integrations/github_enterprise/integration.py

+10
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,12 @@
2929
CommitContextOrganizationOptionKeys,
3030
CommitContextReferrerIds,
3131
CommitContextReferrers,
32+
PullRequestFile,
3233
PullRequestIssue,
3334
)
3435
from sentry.integrations.source_code_management.repository import RepositoryIntegration
3536
from sentry.models.organization import Organization
37+
from sentry.models.pullrequest import PullRequest
3638
from sentry.models.repository import Repository
3739
from sentry.organizations.services.organization.model import RpcOrganization
3840
from sentry.pipeline import NestedPipelineView, Pipeline, PipelineView
@@ -272,6 +274,14 @@ def queue_comment_task(self, pullrequest_id: int, project_id: int) -> None:
272274
def on_create_or_update_comment_error(self, api_error: ApiError, metrics_base: str) -> bool:
273275
raise NotImplementedError
274276

277+
def get_pr_files_safe_for_comment(
278+
self, repo: Repository, pr: PullRequest
279+
) -> list[dict[str, str]]:
280+
raise NotImplementedError
281+
282+
def get_pr_files(self, pr_files: list[dict[str, str]]) -> list[PullRequestFile]:
283+
raise NotImplementedError
284+
275285
def format_open_pr_comment(self, issue_tables: list[str]) -> str:
276286
raise NotImplementedError
277287

‎src/sentry/integrations/gitlab/client.py

+4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
SourceLineInfo,
1919
)
2020
from sentry.integrations.source_code_management.repository import RepositoryClient
21+
from sentry.models.pullrequest import PullRequest
2122
from sentry.models.repository import Repository
2223
from sentry.shared_integrations.client.proxy import IntegrationProxyClient
2324
from sentry.shared_integrations.exceptions import ApiError, ApiUnauthorized
@@ -375,3 +376,6 @@ def get_blame_for_files(
375376
"org_integration_id": self.org_integration_id,
376377
},
377378
)
379+
380+
def get_pullrequest_files(self, repo: Repository, pr: PullRequest) -> Any:
381+
raise NotImplementedError

‎src/sentry/integrations/gitlab/integration.py

+10
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,12 @@
2525
CommitContextOrganizationOptionKeys,
2626
CommitContextReferrerIds,
2727
CommitContextReferrers,
28+
PullRequestFile,
2829
PullRequestIssue,
2930
)
3031
from sentry.integrations.source_code_management.repository import RepositoryIntegration
3132
from sentry.models.organization import Organization
33+
from sentry.models.pullrequest import PullRequest
3234
from sentry.models.repository import Repository
3335
from sentry.pipeline import NestedPipelineView, Pipeline, PipelineView
3436
from sentry.shared_integrations.exceptions import (
@@ -204,6 +206,14 @@ def queue_comment_task(self, pullrequest_id: int, project_id: int) -> None:
204206
def on_create_or_update_comment_error(self, api_error: ApiError, metrics_base: str) -> bool:
205207
raise NotImplementedError
206208

209+
def get_pr_files_safe_for_comment(
210+
self, repo: Repository, pr: PullRequest
211+
) -> list[dict[str, str]]:
212+
raise NotImplementedError
213+
214+
def get_pr_files(self, pr_files: list[dict[str, str]]) -> list[PullRequestFile]:
215+
raise NotImplementedError
216+
207217
def format_open_pr_comment(self, issue_tables: list[str]) -> str:
208218
raise NotImplementedError
209219

‎src/sentry/integrations/source_code_management/commit_context.py

+14
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,16 @@ def run_pr_comment_workflow(
644644

645645
# Open PR Comment Workflow
646646

647+
@abstractmethod
648+
def get_pr_files_safe_for_comment(
649+
self, repo: Repository, pr: PullRequest
650+
) -> list[dict[str, str]]:
651+
raise NotImplementedError
652+
653+
@abstractmethod
654+
def get_pr_files(self, pr_files: list[dict[str, str]]) -> list[PullRequestFile]:
655+
raise NotImplementedError
656+
647657
@abstractmethod
648658
def format_open_pr_comment(self, issue_tables: list[str]) -> str:
649659
raise NotImplementedError
@@ -914,3 +924,7 @@ def update_comment(
914924
@abstractmethod
915925
def get_merge_commit_sha_from_commit(self, repo: Repository, sha: str) -> str | None:
916926
raise NotImplementedError
927+
928+
@abstractmethod
929+
def get_pullrequest_files(self, repo: Repository, pr: PullRequest) -> Any:
930+
raise NotImplementedError

0 commit comments

Comments
 (0)