From 245bce9adb589a02d527db45062c3e9d76d656e5 Mon Sep 17 00:00:00 2001 From: getsentry-bot Date: Thu, 17 Aug 2023 16:11:47 +0000 Subject: [PATCH] Revert "feat(commit-context): Do not create if older than 1 year old (#54866)" This reverts commit 5e70962ba5540b81145cbea9a4bb4a6020eadcbb. Co-authored-by: asottile-sentry <103459774+asottile-sentry@users.noreply.github.com> --- src/sentry/integrations/github/integration.py | 13 ++--- src/sentry/integrations/gitlab/integration.py | 16 +++--- .../integrations/utils/commit_context.py | 8 +-- .../integrations/gitlab/test_integration.py | 3 +- tests/sentry/tasks/test_commit_context.py | 53 +++---------------- tests/sentry/tasks/test_post_process.py | 2 +- 6 files changed, 25 insertions(+), 70 deletions(-) diff --git a/src/sentry/integrations/github/integration.py b/src/sentry/integrations/github/integration.py index ba3a67706bf63f..41d8cabbe74d5e 100644 --- a/src/sentry/integrations/github/integration.py +++ b/src/sentry/integrations/github/integration.py @@ -2,13 +2,12 @@ import logging import re -from datetime import timezone +from datetime import datetime from typing import Any, Collection, Dict, Mapping, Sequence from django.http import HttpResponse from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ -from isodate import parse_datetime from rest_framework.request import Request from sentry import features, options @@ -250,7 +249,9 @@ def get_commit_context( for blame in blame_range if blame.get("startingLine", 0) <= lineno <= blame.get("endingLine", 0) ), - key=lambda blame: parse_datetime(blame.get("commit", {}).get("committedDate")), + key=lambda blame: datetime.strptime( + blame.get("commit", {}).get("committedDate"), "%Y-%m-%dT%H:%M:%SZ" + ), default={}, ) if not commit: @@ -262,13 +263,9 @@ def get_commit_context( if not commitInfo: return None else: - committed_date = parse_datetime(commitInfo.get("committed_date")).astimezone( - timezone.utc - ) - return { "commitId": commitInfo.get("oid"), - "committedDate": committed_date, + "committedDate": commitInfo.get("committedDate"), "commitMessage": commitInfo.get("message"), "commitAuthorName": commitInfo.get("author", {}).get("name"), "commitAuthorEmail": commitInfo.get("author", {}).get("email"), diff --git a/src/sentry/integrations/gitlab/integration.py b/src/sentry/integrations/gitlab/integration.py index 88a3330a1de637..e6d647250dd132 100644 --- a/src/sentry/integrations/gitlab/integration.py +++ b/src/sentry/integrations/gitlab/integration.py @@ -1,13 +1,12 @@ from __future__ import annotations -from datetime import timezone +from datetime import datetime, timezone from typing import Any, Mapping, Sequence from urllib.parse import urlparse from django import forms from django.http import HttpResponse from django.utils.translation import gettext_lazy as _ -from isodate import parse_datetime from rest_framework.request import Request from sentry.identity.gitlab import get_oauth_data, get_user_info @@ -167,10 +166,13 @@ def get_commit_context( except ApiError as e: raise e + date_format_expected = "%Y-%m-%dT%H:%M:%S.%f%z" try: commit = max( blame_range, - key=lambda blame: parse_datetime(blame.get("commit", {}).get("committed_date")), + key=lambda blame: datetime.strptime( + blame.get("commit", {}).get("committed_date"), date_format_expected + ), ) except (ValueError, IndexError): return None @@ -179,11 +181,11 @@ def get_commit_context( if not commitInfo: return None else: - # TODO(nisanthan): Use dateutil.parser.isoparse once on python 3.11 - committed_date = parse_datetime(commitInfo.get("committed_date")).astimezone( - timezone.utc + committed_date = "{}Z".format( + datetime.strptime(commitInfo.get("committed_date"), date_format_expected) + .astimezone(timezone.utc) + .strftime("%Y-%m-%dT%H:%M:%S.%f")[:-3] ) - return { "commitId": commitInfo.get("id"), "committedDate": committed_date, diff --git a/src/sentry/integrations/utils/commit_context.py b/src/sentry/integrations/utils/commit_context.py index e556447d825d85..0c9da764c7b033 100644 --- a/src/sentry/integrations/utils/commit_context.py +++ b/src/sentry/integrations/utils/commit_context.py @@ -1,7 +1,6 @@ from __future__ import annotations import logging -from datetime import datetime, timedelta, timezone from typing import Any, List, Mapping, Sequence, Tuple import sentry_sdk @@ -109,12 +108,7 @@ def find_commit_context_for_event( }, ) - # Only return suspect commits that are less than a year old - if commit_context and is_date_less_than_year(commit_context["committedDate"]): + if commit_context: result.append((commit_context, code_mapping)) return result, installation - - -def is_date_less_than_year(date: datetime): - return date > datetime.now(tz=timezone.utc) - timedelta(days=365) diff --git a/tests/sentry/integrations/gitlab/test_integration.py b/tests/sentry/integrations/gitlab/test_integration.py index 14a0d7fc8136f5..39ed558f8b8d39 100644 --- a/tests/sentry/integrations/gitlab/test_integration.py +++ b/tests/sentry/integrations/gitlab/test_integration.py @@ -5,7 +5,6 @@ import responses from django.core.cache import cache from django.test import override_settings -from isodate import parse_datetime from fixtures.gitlab import GET_COMMIT_RESPONSE, GitLabTestCase from sentry.integrations.gitlab import GitlabIntegrationProvider @@ -426,7 +425,7 @@ def test_get_commit_context(self): commit_context_expected = { "commitId": "d42409d56517157c48bf3bd97d3f75974dde19fb", - "committedDate": parse_datetime("2015-12-18T08:12:22.000Z"), + "committedDate": "2015-12-18T08:12:22.000Z", "commitMessage": "Add installation instructions", "commitAuthorName": "Nisanthan Nanthakumar", "commitAuthorEmail": "nisanthan.nanthakumar@sentry.io", diff --git a/tests/sentry/tasks/test_commit_context.py b/tests/sentry/tasks/test_commit_context.py index 565f63f3a4f09e..7cb3e6882988b8 100644 --- a/tests/sentry/tasks/test_commit_context.py +++ b/tests/sentry/tasks/test_commit_context.py @@ -1,5 +1,4 @@ -from datetime import datetime, timedelta -from datetime import timezone as datetime_timezone +from datetime import timedelta from unittest.mock import Mock, patch import pytest @@ -84,7 +83,7 @@ class TestCommitContext(TestCommitContextMixin): "sentry.integrations.github.GitHubIntegration.get_commit_context", return_value={ "commitId": "asdfwreqr", - "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), + "committedDate": "2023-02-14T11:11Z", "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "admin@localhost", @@ -142,47 +141,11 @@ def test_failed_to_fetch_commit_context_record(self, mock_get_commit_context, mo error_message="integration_failed", ) - @patch("sentry.tasks.commit_context.logger") @patch( "sentry.integrations.github.GitHubIntegration.get_commit_context", return_value={ "commitId": "asdfasdf", - "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=370)), - "commitMessage": "placeholder commit message", - "commitAuthorName": "", - "commitAuthorEmail": "admin@localhost", - }, - ) - def test_found_commit_is_too_old(self, mock_get_commit_context, mock_logger): - with self.tasks(): - assert not GroupOwner.objects.filter(group=self.event.group).exists() - event_frames = get_frame_paths(self.event) - process_commit_context( - event_id=self.event.event_id, - event_platform=self.event.platform, - event_frames=event_frames, - group_id=self.event.group_id, - project_id=self.event.project_id, - ) - - assert mock_logger.info.call_count == 1 - mock_logger.info.assert_called_with( - "process_commit_context.find_commit_context", - extra={ - "event": self.event.event_id, - "group": self.event.group_id, - "organization": self.event.group.project.organization_id, - "reason": "could_not_fetch_commit_context", - "code_mappings_count": 1, - "fallback": True, - }, - ) - - @patch( - "sentry.integrations.github.GitHubIntegration.get_commit_context", - return_value={ - "commitId": "asdfasdf", - "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), + "committedDate": "2023-02-14T11:11Z", "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "admin@localhost", @@ -207,7 +170,7 @@ def test_no_matching_commit_in_db(self, mock_get_commit_context): "sentry.integrations.github.GitHubIntegration.get_commit_context", return_value={ "commitId": "asdfwreqr", - "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), + "committedDate": "2023-02-14T11:11Z", "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "admin@localhost", @@ -292,7 +255,7 @@ def test_no_inapp_frame_in_stacktrace(self, mock_process_suspect_commits): "sentry.integrations.github.GitHubIntegration.get_commit_context", return_value={ "commitId": "somekey", - "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), + "committedDate": "2023-02-14T11:11Z", "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "randomuser@sentry.io", @@ -333,7 +296,7 @@ def test_commit_author_not_in_sentry(self, mock_get_commit_context): "sentry.integrations.github.GitHubIntegration.get_commit_context", return_value={ "commitId": "somekey", - "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), + "committedDate": "2023-02-14T11:11Z", "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "randomuser@sentry.io", @@ -374,7 +337,7 @@ def test_commit_author_no_user(self, mock_get_commit_context, mock_get_users_for "sentry.integrations.github.GitHubIntegration.get_commit_context", return_value={ "commitId": "somekey", - "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), + "committedDate": "2023-02-14T11:11Z", "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "randomuser@sentry.io", @@ -460,7 +423,7 @@ def after_return(self, status, retval, task_id, args, kwargs, einfo): Mock( return_value={ "commitId": "asdfwreqr", - "committedDate": (datetime.now(tz=datetime_timezone.utc) - timedelta(days=7)), + "committedDate": "2023-02-14T11:11Z", "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "admin@localhost", diff --git a/tests/sentry/tasks/test_post_process.py b/tests/sentry/tasks/test_post_process.py index fc7ce446b46f39..24a7811258799f 100644 --- a/tests/sentry/tasks/test_post_process.py +++ b/tests/sentry/tasks/test_post_process.py @@ -1303,7 +1303,7 @@ def test_issue_owners_should_ratelimit(self, logger): class ProcessCommitsTestMixin(BasePostProgressGroupMixin): github_blame_return_value = { "commitId": "asdfwreqr", - "committedDate": (datetime.now(timezone.utc) - timedelta(days=2)), + "committedDate": "", "commitMessage": "placeholder commit message", "commitAuthorName": "", "commitAuthorEmail": "admin@localhost",