Skip to content
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

ref(releases): Move code to a new function #81208

Closed
wants to merge 5 commits into from
Closed
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
220 changes: 111 additions & 109 deletions src/sentry/api/helpers/group_index/update.py
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The split view is better for reviewing this PR:
image

Original file line number Diff line number Diff line change
Expand Up @@ -299,128 +299,38 @@ def handle_resolve_in_release(
user: User | RpcUser,
result: MutableMapping[str, Any],
) -> tuple[dict[str, Any], GroupResolution.Type | None] | Response:
res_type = None
release = None
commit = None
self_assign_issue = "0"
if acting_user:
user_options = user_option_service.get_many(
filter={"user_ids": [acting_user.id], "keys": ["self_assign_issue"]}
)
if user_options:
self_assign_issue = user_options[0].value
res_status = None
if status == "resolvedInNextRelease" or status_details.get("inNextRelease"):
# TODO(jess): We may want to support this for multi project, but punting on it for now
if len(projects) > 1:
return Response(
{"detail": "Cannot set resolved in next release for multiple projects."},
status=400,
)
# may not be a release yet
release = status_details.get("inNextRelease") or get_release_to_resolve_by(projects[0])

activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value
activity_data = {
# no version yet
"version": ""
}

serialized_user = user_service.serialize_many(filter=dict(user_ids=[user.id]), as_user=user)
new_status_details = {
"inNextRelease": True,
}
if serialized_user:
new_status_details["actor"] = serialized_user[0]
res_type = GroupResolution.Type.in_next_release
res_type_str = "in_next_release"
res_status = GroupResolution.Status.pending
elif status_details.get("inUpcomingRelease"):
if len(projects) > 1:
return Response(
{"detail": "Cannot set resolved in upcoming release for multiple projects."},
status=400,
)
release = status_details.get("inUpcomingRelease") or most_recent_release(projects[0])
activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value
activity_data = {"version": ""}

serialized_user = user_service.serialize_many(filter=dict(user_ids=[user.id]), as_user=user)
new_status_details = {
"inUpcomingRelease": True,
}
if serialized_user:
new_status_details["actor"] = serialized_user[0]
res_type = GroupResolution.Type.in_upcoming_release
res_type_str = "in_upcoming_release"
res_status = GroupResolution.Status.pending
elif status_details.get("inRelease"):
# TODO(jess): We could update validation to check if release
# applies to multiple projects, but I think we agreed to punt
# on this for now
if len(projects) > 1:
return Response(
{"detail": "Cannot set resolved in release for multiple projects."}, status=400
)
release = status_details["inRelease"]
activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value
activity_data = {
# no version yet
"version": release.version
}

serialized_user = user_service.serialize_many(filter=dict(user_ids=[user.id]), as_user=user)
new_status_details = {
"inRelease": release.version,
}
if serialized_user:
new_status_details["actor"] = serialized_user[0]
res_type = GroupResolution.Type.in_release
res_type_str = "in_release"
res_status = GroupResolution.Status.resolved
elif status_details.get("inCommit"):
# TODO(jess): Same here, this is probably something we could do, but
# punting for now.
if (
status == "resolvedInNextRelease"
or status_details.get("inNextRelease")
or status_details.get("inUpcomingRelease")
or status_details.get("inRelease")
):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The four if statements prevent the usage for multiple projects:

# TODO(jess): We may want to support this for multi project, but punting on it for now
if len(projects) > 1:
return Response(
{"detail": "Cannot set resolved in next release for multiple projects."},
status=400,
)

# TODO(jess): We may want to support this for multi project, but punting on it for now
if len(projects) > 1:
return Response(
{"detail": "Cannot set resolved in commit for multiple projects."}, status=400
{"detail": "Cannot set resolve in release for multiple projects."}, status=400
)
commit = status_details["inCommit"]
activity_type = ActivityType.SET_RESOLVED_IN_COMMIT.value
activity_data = {"commit": commit.id}
serialized_user = user_service.serialize_many(filter=dict(user_ids=[user.id]), as_user=user)

new_status_details = {
"inCommit": serialize(commit, user),
}
if serialized_user:
new_status_details["actor"] = serialized_user[0]
res_type_str = "in_commit"
else:
res_type_str = "now"
activity_type = ActivityType.SET_RESOLVED.value
activity_data = {}
new_status_details = {}
(
activity_type,
activity_data,
new_status_details,
res_type,
res_type_str,
res_status,
commit,
release,
) = resolve_in_release_helper(status, status_details, projects, user)

metrics.incr("group.resolved", instance=res_type_str, skip_internal=True)

# if we've specified a commit, let's see if its already been released
# this will allow us to associate the resolution to a release as if we
# were simply using 'inRelease' above
# Note: this is different than the way commit resolution works on deploy
# creation, as a given deploy is connected to an explicit release, and
# in this case we're simply choosing the most recent release which contains
# the commit.
if commit and not release:
# TODO(jess): If we support multiple projects for release / commit resolution,
# we need to update this to find the release for each project (we shouldn't assume
# it's the same)
try:
release = most_recent_release_matching_commit(projects, commit)
res_type = GroupResolution.Type.in_release
res_status = GroupResolution.Status.resolved
except IndexError:
release = None
for group in group_list:
with transaction.atomic(router.db_for_write(Group)):
process_group_resolution(
Expand All @@ -439,7 +349,7 @@ def handle_resolve_in_release(
)

issue_resolved.send_robust(
organization_id=projects[0].organization_id,
organization_id=group.project.organization_id,
user=(acting_user or user),
group=group,
project=project_lookup[group.project_id],
Expand Down Expand Up @@ -619,6 +529,98 @@ def process_group_resolution(
transaction.on_commit(lambda: activity.send_notification(), router.db_for_write(Group))


def resolve_in_release_helper(
status: str,
status_details: Mapping[str, Any],
projects: Sequence[Project],
user: User,
) -> tuple[
int,
dict[str, Any],
dict[str, Any],
GroupResolution.Type,
str,
GroupResolution.Status,
Commit | None,
Release | None,
]:
# The default values for the resolution
res_type_str = "now"
activity_type = ActivityType.SET_RESOLVED.value
activity_data = {}
new_status_details = {}

commit = None
release = None
serialized_user = user_service.serialize_many(filter=dict(user_ids=[user.id]), as_user=user)
if serialized_user:
new_status_details["actor"] = serialized_user[0]

if status == "resolvedInNextRelease" or status_details.get("inNextRelease"):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I copy/paste the original code the only differences are these:

diff --git a/src/sentry/api/helpers/group_index/update.py b/src/sentry/api/helpers/group_index/update.py
index 4d872033722..8de70a96876 100644
--- a/src/sentry/api/helpers/group_index/update.py
+++ b/src/sentry/api/helpers/group_index/update.py
@@ -548,6 +548,12 @@ def resolve_in_release_helper(
     release = None
 
     if status == "resolvedInNextRelease" or status_details.get("inNextRelease"):
+        # TODO(jess): We may want to support this for multi project, but punting on it for now
+        if len(projects) > 1:
+            return Response(
+                {"detail": "Cannot set resolved in next release for multiple projects."},
+                status=400,
+            )
         # may not be a release yet
         release = status_details.get("inNextRelease") or get_release_to_resolve_by(projects[0])
 
@@ -567,6 +573,11 @@ def resolve_in_release_helper(
         res_type_str = "in_next_release"
         res_status = GroupResolution.Status.pending
     elif status_details.get("inUpcomingRelease"):
+        if len(projects) > 1:
+            return Response(
+                {"detail": "Cannot set resolved in upcoming release for multiple projects."},
+                status=400,
+            )
         release = status_details.get("inUpcomingRelease") or most_recent_release(projects[0])
         activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value
         activity_data = {"version": ""}
@@ -581,6 +592,13 @@ def resolve_in_release_helper(
         res_type_str = "in_upcoming_release"
         res_status = GroupResolution.Status.pending
     elif status_details.get("inRelease"):
+        # TODO(jess): We could update validation to check if release
+        # applies to multiple projects, but I think we agreed to punt
+        # on this for now
+        if len(projects) > 1:
+            return Response(
+                {"detail": "Cannot set resolved in release for multiple projects."}, status=400
+            )
         release = status_details["inRelease"]
         activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value
         activity_data = {
@@ -598,6 +616,12 @@ def resolve_in_release_helper(
         res_type_str = "in_release"
         res_status = GroupResolution.Status.resolved
     elif status_details.get("inCommit"):
+        # TODO(jess): Same here, this is probably something we could do, but
+        # punting for now.
+        if len(projects) > 1:
+            return Response(
+                {"detail": "Cannot set resolved in commit for multiple projects."}, status=400
+            )
         commit = status_details["inCommit"]
         activity_type = ActivityType.SET_RESOLVED_IN_COMMIT.value
         activity_data = {"commit": commit.id}

# may not be a release yet
release = status_details.get("inNextRelease") or get_release_to_resolve_by(projects[0])
activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value
activity_data = {"version": ""} # no version yet
new_status_details = {"inNextRelease": True}
res_type = GroupResolution.Type.in_next_release
res_type_str = "in_next_release"
res_status = GroupResolution.Status.pending

elif status_details.get("inUpcomingRelease"):
release = status_details.get("inUpcomingRelease") or most_recent_release(projects[0])
activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value
activity_data = {"version": ""}
new_status_details = {"inUpcomingRelease": True}
res_type = GroupResolution.Type.in_upcoming_release
res_type_str = "in_upcoming_release"
res_status = GroupResolution.Status.pending

elif status_details.get("inRelease"):
release = status_details["inRelease"]
activity_type = ActivityType.SET_RESOLVED_IN_RELEASE.value
activity_data = {"version": release.version}
new_status_details = {"inRelease": release.version}
res_type = GroupResolution.Type.in_release
res_type_str = "in_release"
res_status = GroupResolution.Status.resolved

elif status_details.get("inCommit"):
commit = status_details["inCommit"]
activity_type = ActivityType.SET_RESOLVED_IN_COMMIT.value
activity_data = {"commit": commit.id}
new_status_details = {"inCommit": serialize(commit, user)}
res_type_str = "in_commit"

# if we've specified a commit, let's see if its already been released
# this will allow us to associate the resolution to a release as if we
# were simply using 'inRelease' above
# Note: this is different than the way commit resolution works on deploy
# creation, as a given deploy is connected to an explicit release, and
# in this case we're simply choosing the most recent release which contains
# the commit.
if commit and not release:
# TODO(jess): If we support multiple projects for release / commit resolution,
# we need to update this to find the release for each project (we shouldn't assume
# it's the same)
try:
release = most_recent_release_matching_commit(projects, commit)
res_type = GroupResolution.Type.in_release
res_status = GroupResolution.Status.resolved
except IndexError:
release = None
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines come from this block:

# if we've specified a commit, let's see if its already been released
# this will allow us to associate the resolution to a release as if we
# were simply using 'inRelease' above
# Note: this is different than the way commit resolution works on deploy
# creation, as a given deploy is connected to an explicit release, and
# in this case we're simply choosing the most recent release which contains
# the commit.
if commit and not release:
# TODO(jess): If we support multiple projects for release / commit resolution,
# we need to update this to find the release for each project (we shouldn't assume
# it's the same)
try:
release = most_recent_release_matching_commit(projects, commit)
res_type = GroupResolution.Type.in_release
res_status = GroupResolution.Status.resolved
except IndexError:
release = None


return (
activity_type,
activity_data,
new_status_details,
res_type,
res_type_str,
res_status,
commit,
release,
)


def merge_groups(
group_list: Sequence[Group],
project_lookup: Mapping[int, Project],
Expand Down
Loading