Repository navigation
[benchmark][Python] feat(ecosystem): Implement cross-system issue synchronization #12
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
base: benchmark/sentry/pr-77754-base
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,30 +1,45 @@ | ||
| from sentry.eventstore.models import GroupEvent | ||
| from sentry.integrations.base import IntegrationInstallation | ||
| from sentry.integrations.models.external_issue import ExternalIssue | ||
| from sentry.integrations.services.assignment_source import AssignmentSource | ||
| from sentry.integrations.services.integration import integration_service | ||
| from sentry.integrations.tasks.sync_status_inbound import ( | ||
| sync_status_inbound as sync_status_inbound_task, | ||
|
|
||
|
|
||
| class IssueBasicIntegration(IntegrationInstallation, ABC): | ||
| def should_sync(self, attribute): | ||
| def should_sync(self, attribute, sync_source: AssignmentSource | None = None): | ||
| return False | ||
|
|
||
| def get_group_title(self, group, event, **kwargs): | ||
| outbound_assignee_key: ClassVar[str | None] = None | ||
| inbound_assignee_key: ClassVar[str | None] = None | ||
|
|
||
| def should_sync(self, attribute: str) -> bool: | ||
| def should_sync(self, attribute: str, sync_source: AssignmentSource | None = None) -> bool: | ||
| key = getattr(self, f"{attribute}_key", None) | ||
| if key is None or self.org_integration is None: | ||
| return False | ||
|
|
||
| # Check that the assignment source isn't this same integration in order to | ||
| # prevent sync-cycles from occurring. This should still allow other | ||
| # integrations to propagate changes outward. | ||
| if sync_source and sync_source.integration_id == self.org_integration.integration_id: | ||
| return False | ||
|
|
||
| value: bool = self.org_integration.config.get(key, False) | ||
| return value | ||
|
|
||
| raise NotImplementedError | ||
|
|
||
| @abstractmethod | ||
| def sync_status_outbound(self, external_issue, is_resolved, project_id, **kwargs): | ||
| def sync_status_outbound( | ||
| self, | ||
| external_issue, | ||
| is_resolved, | ||
| project_id, | ||
| assignment_source: AssignmentSource | None = None, | ||
| **kwargs, | ||
| ): | ||
| """ | ||
| Propagate a sentry issue's status to a linked issue's status. | ||
| """ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from dataclasses import asdict, dataclass | ||
| from datetime import datetime | ||
| from typing import TYPE_CHECKING, Any | ||
|
|
||
| from django.utils import timezone | ||
|
|
||
| if TYPE_CHECKING: | ||
| from sentry.integrations.models import Integration | ||
| from sentry.integrations.services.integration import RpcIntegration | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class AssignmentSource: | ||
| source_name: str | ||
| integration_id: int | ||
| queued: datetime = timezone.now() | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 Значение по умолчанию В frozen-датаклассе |
||
|
|
||
| @classmethod | ||
| def from_integration(cls, integration: Integration | RpcIntegration) -> AssignmentSource: | ||
| return AssignmentSource( | ||
| source_name=integration.name, | ||
| integration_id=integration.id, | ||
| ) | ||
|
|
||
| def to_dict(self) -> dict[str, Any]: | ||
| return asdict(self) | ||
|
|
||
| @classmethod | ||
| def from_dict(cls, input_dict: dict[str, Any]) -> AssignmentSource | None: | ||
| try: | ||
| return cls(**input_dict) | ||
| except (ValueError, TypeError): | ||
| return None | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,36 @@ | ||
| from typing import Any | ||
|
|
||
| from sentry import analytics, features | ||
| from sentry.integrations.models.external_issue import ExternalIssue | ||
| from sentry.integrations.models.integration import Integration | ||
| from sentry.integrations.services.assignment_source import AssignmentSource | ||
| from sentry.integrations.services.integration import integration_service | ||
| from sentry.models.organization import Organization | ||
| from sentry.silo.base import SiloMode | ||
| Organization.DoesNotExist, | ||
| ) | ||
| ) | ||
| def sync_assignee_outbound(external_issue_id: int, user_id: int | None, assign: bool) -> None: | ||
| def sync_assignee_outbound( | ||
| external_issue_id: int, | ||
| user_id: int | None, | ||
| assign: bool, | ||
| assignment_source_dict: dict[str, Any] | None = None, | ||
| ) -> None: | ||
| # Sync Sentry assignee to an external issue. | ||
| external_issue = ExternalIssue.objects.get(id=external_issue_id) | ||
|
|
||
| ): | ||
| return | ||
|
|
||
| if installation.should_sync("outbound_assignee"): | ||
| parsed_assignment_source = ( | ||
| AssignmentSource.from_dict(assignment_source_dict) if assignment_source_dict else None | ||
| ) | ||
| if installation.should_sync("outbound_assignee", parsed_assignment_source): | ||
| # Assume unassign if None. | ||
| user = user_service.get_user(user_id) if user_id else None | ||
| installation.sync_assignee_outbound(external_issue, user, assign=assign) | ||
| installation.sync_assignee_outbound( | ||
| external_issue, user, assign=assign, assignment_source=parsed_assignment_source | ||
| ) | ||
| analytics.record( | ||
| "integration.issue.assignee.synced", | ||
| provider=integration.provider, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,32 +1,53 @@ | ||
| from typing import TYPE_CHECKING | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Потеря типа Функция |
||
|
|
||
| from sentry import features | ||
| from sentry.integrations.services.assignment_source import AssignmentSource | ||
| from sentry.integrations.services.integration import integration_service | ||
| from sentry.integrations.tasks.sync_assignee_outbound import sync_assignee_outbound | ||
| from sentry.models.group import Group | ||
|
|
||
| if not assign: | ||
| for group in affected_groups: | ||
| GroupAssignee.objects.deassign(group) | ||
| GroupAssignee.objects.deassign( | ||
| group, | ||
| assignment_source=AssignmentSource.from_integration(integration), | ||
| ) | ||
|
|
||
| return affected_groups | ||
|
|
||
| users = user_service.get_many_by_email(emails=[email], is_verified=True) | ||
| user_id = get_user_id(projects_by_user, group) | ||
| user = users_by_id.get(user_id) | ||
| if user: | ||
| GroupAssignee.objects.assign(group, user) | ||
| GroupAssignee.objects.assign( | ||
| group, | ||
| user, | ||
| assignment_source=AssignmentSource.from_integration(integration), | ||
| ) | ||
| groups_assigned.append(group) | ||
| else: | ||
| logger.info("assignee-not-found-inbound", extra=log_context) | ||
| return groups_assigned | ||
|
|
||
|
|
||
| def sync_group_assignee_outbound(group: Group, user_id: int | None, assign: bool = True) -> None: | ||
| def sync_group_assignee_outbound( | ||
| group: Group, | ||
| user_id: int | None, | ||
| assign: bool = True, | ||
| assignment_source: AssignmentSource | None = None, | ||
| ) -> None: | ||
| from sentry.models.grouplink import GroupLink | ||
|
|
||
| external_issue_ids = GroupLink.objects.filter( | ||
|
|
||
| for external_issue_id in external_issue_ids: | ||
| sync_assignee_outbound.apply_async( | ||
| kwargs={"external_issue_id": external_issue_id, "user_id": user_id, "assign": assign} | ||
| kwargs={ | ||
| "external_issue_id": external_issue_id, | ||
| "user_id": user_id, | ||
| "assign": assign, | ||
| "assignment_source_dict": assignment_source.to_dict() | ||
| if assignment_source | ||
| else None, | ||
| } | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,32 +1,39 @@ | ||
| from sentry.db.models import FlexibleForeignKey, Model, region_silo_model, sane_repr | ||
| from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey | ||
| from sentry.db.models.manager.base import BaseManager | ||
| from sentry.integrations.services.assignment_source import AssignmentSource | ||
| from sentry.models.grouphistory import GroupHistoryStatus, record_group_history | ||
| from sentry.models.groupowner import GroupOwner | ||
| from sentry.models.groupsubscription import GroupSubscription | ||
| create_only: bool = False, | ||
| extra: dict[str, str] | None = None, | ||
| force_autoassign: bool = False, | ||
| assignment_source: AssignmentSource | None = None, | ||
| ): | ||
| from sentry.integrations.utils import sync_group_assignee_outbound | ||
| from sentry.models.activity import Activity | ||
| if assignee_type == "user" and features.has( | ||
| "organizations:integrations-issue-sync", group.organization, actor=acting_user | ||
| ): | ||
| sync_group_assignee_outbound(group, assigned_to.id, assign=True) | ||
| sync_group_assignee_outbound( | ||
| group, assigned_to.id, assign=True, assignment_source=assignment_source | ||
| ) | ||
|
|
||
| if not created: # aka re-assignment | ||
| self.remove_old_assignees(group, assignee, assigned_to_id, assignee_type) | ||
| acting_user: User | RpcUser | None = None, | ||
| assigned_to: Team | RpcUser | None = None, | ||
| extra: dict[str, str] | None = None, | ||
| assignment_source: AssignmentSource | None = None, | ||
| ) -> None: | ||
| from sentry.integrations.utils import sync_group_assignee_outbound | ||
| from sentry.models.activity import Activity | ||
| if features.has( | ||
| "organizations:integrations-issue-sync", group.organization, actor=acting_user | ||
| ): | ||
| sync_group_assignee_outbound(group, None, assign=False) | ||
| sync_group_assignee_outbound( | ||
| group, None, assign=False, assignment_source=assignment_source | ||
| ) | ||
|
|
||
| issue_unassigned.send_robust( | ||
| project=group.project, group=group, user=acting_user, sender=self.__class__ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| from typing import Any | ||
|
|
||
| from sentry.integrations.services.assignment_source import AssignmentSource | ||
| from sentry.testutils.cases import TestCase | ||
|
|
||
|
|
||
| class TestAssignmentSource(TestCase): | ||
| def test_from_dict_empty_array(self): | ||
| data: dict[str, Any] = {} | ||
| result = AssignmentSource.from_dict(data) | ||
| assert result is None | ||
|
|
||
| def test_from_dict_inalid_data(self): | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 Опечатка в имени теста: Имя тестового метода содержит опечатку — |
||
| data = { | ||
| "foo": "bar", | ||
| } | ||
|
|
||
| result = AssignmentSource.from_dict(data) | ||
| assert result is None | ||
|
|
||
| def test_from_dict_valid_data(self): | ||
| data = {"source_name": "foo-source", "integration_id": 123} | ||
|
|
||
| result = AssignmentSource.from_dict(data) | ||
| assert result is not None | ||
| assert result.source_name == "foo-source" | ||
| assert result.integration_id == 123 | ||
|
|
||
| def test_to_dict(self): | ||
| source = AssignmentSource( | ||
| source_name="foo-source", | ||
| integration_id=123, | ||
| ) | ||
|
|
||
| result = source.to_dict() | ||
| assert result.get("queued") is not None | ||
| assert result.get("source_name") == "foo-source" | ||
| assert result.get("integration_id") == 123 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,93 @@ | ||
|
|
||
| from sentry.integrations.example.integration import ExampleIntegration | ||
| from sentry.integrations.models.external_issue import ExternalIssue | ||
| from sentry.integrations.services.assignment_source import AssignmentSource | ||
| from sentry.integrations.utils import sync_group_assignee_inbound | ||
| from sentry.models.activity import Activity | ||
| from sentry.models.groupassignee import GroupAssignee | ||
|
|
||
| with self.feature({"organizations:integrations-issue-sync": True}): | ||
| with self.tasks(): | ||
| GroupAssignee.objects.assign(self.group, self.user) | ||
| GroupAssignee.objects.assign( | ||
| self.group, | ||
| self.user, | ||
| ) | ||
|
|
||
| mock_sync_assignee_outbound.assert_called_with( | ||
| external_issue, user_service.get_user(self.user.id), assign=True | ||
| external_issue, | ||
| user_service.get_user(self.user.id), | ||
| assign=True, | ||
| assignment_source=None, | ||
| ) | ||
|
|
||
| assert GroupAssignee.objects.filter( | ||
| project=self.group.project, | ||
| group=self.group, | ||
| user_id=self.user.id, | ||
| team__isnull=True, | ||
| ).exists() | ||
|
|
||
| activity = Activity.objects.get( | ||
| project=self.group.project, group=self.group, type=ActivityType.ASSIGNED.value | ||
| ) | ||
|
|
||
| assert activity.data["assignee"] == str(self.user.id) | ||
| assert activity.data["assigneeEmail"] == self.user.email | ||
| assert activity.data["assigneeType"] == "user" | ||
|
|
||
| @mock.patch.object(ExampleIntegration, "sync_assignee_outbound") | ||
| def test_assignee_sync_outbound_assign_with_matching_source_integration( | ||
| self, mock_sync_assignee_outbound | ||
| ): | ||
| group = self.group | ||
| integration = self.create_integration( | ||
| organization=group.organization, | ||
| external_id="123456", | ||
| provider="example", | ||
| oi_params={ | ||
| "config": { | ||
| "sync_comments": True, | ||
| "sync_status_outbound": True, | ||
| "sync_status_inbound": True, | ||
| "sync_assignee_outbound": True, | ||
| "sync_assignee_inbound": True, | ||
| } | ||
| }, | ||
| ) | ||
|
|
||
| external_issue = ExternalIssue.objects.create( | ||
| organization_id=group.organization.id, integration_id=integration.id, key="APP-123" | ||
| ) | ||
|
|
||
| GroupLink.objects.create( | ||
| group_id=group.id, | ||
| project_id=group.project_id, | ||
| linked_type=GroupLink.LinkedType.issue, | ||
| linked_id=external_issue.id, | ||
| relationship=GroupLink.Relationship.references, | ||
| ) | ||
|
|
||
| with self.feature({"organizations:integrations-issue-sync": True}): | ||
| with self.tasks(): | ||
| # Assert that we don't perform an outbound assignment if | ||
| # the source of the assignment is the same target integration | ||
| GroupAssignee.objects.assign( | ||
| self.group, | ||
| self.user, | ||
| assignment_source=AssignmentSource.from_integration(integration), | ||
| ) | ||
|
|
||
| mock_sync_assignee_outbound.assert_not_called() | ||
|
|
||
| assert GroupAssignee.objects.filter( | ||
| project=self.group.project, | ||
| group=self.group, | ||
| with self.feature({"organizations:integrations-issue-sync": True}): | ||
| with self.tasks(): | ||
| GroupAssignee.objects.deassign(self.group) | ||
| mock_sync_assignee_outbound.assert_called_with(external_issue, None, assign=False) | ||
| mock_sync_assignee_outbound.assert_called_with( | ||
| external_issue, None, assign=False, assignment_source=None | ||
| ) | ||
|
|
||
| assert not GroupAssignee.objects.filter( | ||
| project=self.group.project, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔵 Базовый метод
should_syncигнорирует параметрsync_sourceСигнатура
IssueBasicIntegration.should_syncрасширена параметромsync_source: AssignmentSource | None = None, но тело метода безусловно возвращаетFalse, не используя новый аргумент. Это допустимо как no-op базовая реализация, но отсутствует docstring или комментарий, поясняющий, что логика предотвращения циклов реализована только в подклассах. Будущие читатели могут ошибочно предположить, что базовый класс выполняет проверку циклов. Рекомендуется добавить комментарий с описанием намерения.