Skip to content

Commit bf4d35e

Browse files
authored
chore(ecosystem): Bitbucket SLOs for get_repos (#95373)
It's a bit odd, but the exception raised here doesn't need to adjust our SLOs just to fail to the user, especially since we actually care about the API error. Maybe theres tooling to designate exception that would be processed as halts, but I couldn't find it. Instead, I essentially wrapped the business logic in a try/catch to produce a failure I could bubble up after the lifecycle could be closed. We still capture the failure in the internal function, so nothing should change coverage wise.
1 parent 51198ac commit bf4d35e

File tree

5 files changed

+91
-29
lines changed

5 files changed

+91
-29
lines changed

src/sentry/integrations/bitbucket/issues.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ def get_create_issue_config(
6868
org = org_context.organization
6969

7070
params = kwargs.pop("params", {})
71-
default_repo, repo_choices = self.get_repository_choices(group, params, **kwargs)
71+
default_repo, repo_choices = self.get_repository_choices(group, params)
7272

7373
autocomplete_url = reverse(
7474
"sentry-extensions-bitbucket-search", args=[org.slug, self.model.id]
@@ -111,7 +111,7 @@ def get_create_issue_config(
111111

112112
def get_link_issue_config(self, group: Group, **kwargs) -> list[dict[str, Any]]:
113113
params = kwargs.pop("params", {})
114-
default_repo, repo_choices = self.get_repository_choices(group, params, **kwargs)
114+
default_repo, repo_choices = self.get_repository_choices(group, params)
115115

116116
org = group.organization
117117
autocomplete_url = reverse(

src/sentry/integrations/github/issues.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ def get_create_issue_config(
162162
org = org_context.organization
163163

164164
params = kwargs.pop("params", {})
165-
default_repo, repo_choices = self.get_repository_choices(group, params, **kwargs)
165+
default_repo, repo_choices = self.get_repository_choices(group, params)
166166

167167
assignees = self.get_allowed_assignees(default_repo) if default_repo else []
168168
labels: Sequence[tuple[str, str]] = []
@@ -238,7 +238,7 @@ def create_issue(self, data: Mapping[str, Any], **kwargs: Any) -> Mapping[str, A
238238

239239
def get_link_issue_config(self, group: Group, **kwargs: Any) -> list[dict[str, Any]]:
240240
params = kwargs.pop("params", {})
241-
default_repo, repo_choices = self.get_repository_choices(group, params, **kwargs)
241+
default_repo, repo_choices = self.get_repository_choices(group, params)
242242

243243
org = group.organization
244244
autocomplete_url = reverse(

src/sentry/integrations/gitlab/issues.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ def get_projects_and_default(self, group: Group, params: Mapping[str, Any], **kw
4545
params_mut = dict(params)
4646
params_mut["repo"] = params.get("project") or defaults.get("project")
4747

48-
default_project, project_choices = self.get_repository_choices(group, params_mut, **kwargs)
48+
default_project, project_choices = self.get_repository_choices(group, params_mut)
4949
return default_project, project_choices
5050

5151
def create_default_repo_choice(self, default_repo):

src/sentry/integrations/source_code_management/issues.py

Lines changed: 47 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,14 @@
1010
SCMIntegrationInteractionType,
1111
)
1212
from sentry.integrations.source_code_management.repository import BaseRepositoryIntegration
13+
from sentry.integrations.utils.metrics import EventLifecycle
1314
from sentry.models.group import Group
1415
from sentry.shared_integrations.exceptions import ApiError, IntegrationError
1516

17+
SOURCE_CODE_ISSUE_HALT_PATTERNS = {
18+
"No workspace with identifier", # Bitbucket
19+
}
20+
1621

1722
class SourceCodeIssueIntegration(IssueBasicIntegration, BaseRepositoryIntegration, ABC):
1823
def record_event(self, event: SCMIntegrationInteractionType):
@@ -23,35 +28,53 @@ def record_event(self, event: SCMIntegrationInteractionType):
2328
org_integration=self.org_integration,
2429
)
2530

26-
def get_repository_choices(self, group: Group | None, params: Mapping[str, Any], **kwargs):
27-
"""
28-
Returns the default repository and a set/subset of repositories of associated with the installation
29-
"""
30-
with self.record_event(SCMIntegrationInteractionType.GET_REPOSITORY_CHOICES).capture():
31-
try:
32-
repos = self.get_repositories()
33-
except ApiError:
34-
raise IntegrationError("Unable to retrieve repositories. Please try again later.")
31+
def _get_repository_choices(
32+
self, *, group: Group | None, params: Mapping[str, Any], lifecycle: EventLifecycle
33+
):
34+
try:
35+
repos = self.get_repositories()
36+
except ApiError as exc:
37+
if any(pattern in str(exc) for pattern in SOURCE_CODE_ISSUE_HALT_PATTERNS):
38+
lifecycle.record_halt(exc)
3539
else:
36-
repo_choices = [(repo["identifier"], repo["name"]) for repo in repos]
40+
lifecycle.record_failure(exc)
41+
raise IntegrationError("Unable to retrieve repositories. Please try again later.")
42+
else:
43+
repo_choices = [(repo["identifier"], repo["name"]) for repo in repos]
3744

38-
defaults = self.get_project_defaults(group.project_id) if group else {}
39-
repo = params.get("repo") or defaults.get("repo")
45+
defaults = self.get_project_defaults(group.project_id) if group else {}
46+
repo = params.get("repo") or defaults.get("repo")
4047

41-
try:
42-
default_repo = repo or repo_choices[0][0]
43-
except IndexError:
44-
return "", repo_choices
48+
try:
49+
default_repo = repo or repo_choices[0][0]
50+
except IndexError:
51+
return "", repo_choices
4552

46-
# If a repo has been selected outside of the default list of
47-
# repos, stick it onto the front of the list so that it can be
48-
# selected.
49-
try:
50-
next(True for r in repo_choices if r[0] == default_repo)
51-
except StopIteration:
52-
repo_choices.insert(0, self.create_default_repo_choice(default_repo))
53+
# If a repo has been selected outside of the default list of
54+
# repos, stick it onto the front of the list so that it can be
55+
# selected.
56+
try:
57+
next(True for r in repo_choices if r[0] == default_repo)
58+
except StopIteration:
59+
repo_choices.insert(0, self.create_default_repo_choice(default_repo))
60+
61+
return default_repo, repo_choices
5362

54-
return default_repo, repo_choices
63+
def get_repository_choices(self, group: Group | None, params: Mapping[str, Any]):
64+
"""
65+
Returns the default repository and a set/subset of repositories of associated with the installation
66+
"""
67+
user_facing_error = None
68+
with self.record_event(
69+
SCMIntegrationInteractionType.GET_REPOSITORY_CHOICES
70+
).capture() as lifecycle:
71+
try:
72+
return self._get_repository_choices(group=group, params=params, lifecycle=lifecycle)
73+
except IntegrationError as exc:
74+
user_facing_error = exc
75+
# Now that we're outside the lifecycle, we can raise the user facing error
76+
if user_facing_error:
77+
raise user_facing_error
5578

5679
# TODO(saif): Make private and move all usages over to `get_defaults`
5780
def get_project_defaults(self, project_id):

tests/sentry/integrations/bitbucket/test_integration.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
1+
from unittest.mock import patch
12
from urllib.parse import quote, urlencode
23

4+
import pytest
35
import responses
46
from django.urls import reverse
57

68
from sentry.integrations.bitbucket.integration import BitbucketIntegrationProvider
79
from sentry.integrations.models.integration import Integration
810
from sentry.models.repository import Repository
11+
from sentry.shared_integrations.exceptions import IntegrationError
912
from sentry.silo.base import SiloMode
1013
from sentry.testutils.cases import APITestCase
1114
from sentry.testutils.silo import assume_test_silo_mode, control_silo_test
@@ -203,3 +206,39 @@ def test_extract_source_path_from_source_url(self):
203206
installation.extract_source_path_from_source_url(repo, source_url)
204207
== "src/sentry/integrations/bitbucket/integration.py"
205208
)
209+
210+
@responses.activate
211+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_failure")
212+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_halt")
213+
def test_get_repository_choices_halt_lifecycle(self, mock_record_halt, mock_record_failure):
214+
responses.add(
215+
responses.GET,
216+
"https://api.bitbucket.org/2.0/repositories/sentryuser",
217+
status=404,
218+
json={"error": {"message": "No workspace with identifier sentryuser"}},
219+
)
220+
installation = self.integration.get_installation(self.organization.id)
221+
with pytest.raises(
222+
IntegrationError, match="Unable to retrieve repositories. Please try again later."
223+
):
224+
installation.get_repository_choices(None, {})
225+
assert mock_record_halt.call_count == 1
226+
assert mock_record_failure.call_count == 0
227+
228+
@responses.activate
229+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_failure")
230+
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_halt")
231+
def test_get_repository_choices_failure_lifecycle(self, mock_record_halt, mock_record_failure):
232+
responses.add(
233+
responses.GET,
234+
"https://api.bitbucket.org/2.0/repositories/sentryuser",
235+
status=404,
236+
json={"error": {"message": "Some other error, sentry is responsible"}},
237+
)
238+
installation = self.integration.get_installation(self.organization.id)
239+
with pytest.raises(
240+
IntegrationError, match="Unable to retrieve repositories. Please try again later."
241+
):
242+
installation.get_repository_choices(None, {})
243+
assert mock_record_halt.call_count == 0
244+
assert mock_record_failure.call_count == 1

0 commit comments

Comments
 (0)