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.

Sign up for GitHub

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: make create or update typesafe #75149

Merged
merged 1 commit into from
Jul 29, 2024
Merged

ref: make create or update typesafe #75149

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
24 changes: 13 additions & 11 deletions src/sentry/models/release.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import re
from collections.abc import Mapping, Sequence
from time import time
from typing import ClassVar
from typing import ClassVar, TypedDict

import orjson
import sentry_sdk
Expand Down Expand Up @@ -36,6 +36,7 @@
from sentry.locks import locks
from sentry.models.activity import Activity
from sentry.models.artifactbundle import ArtifactBundle
from sentry.models.commitauthor import CommitAuthor
from sentry.models.commitfilechange import CommitFileChange
from sentry.models.grouphistory import GroupHistoryStatus, record_group_history
from sentry.models.groupinbox import GroupInbox, GroupInboxRemoveAction, remove_group_from_inbox
Expand Down Expand Up @@ -178,6 +179,12 @@ def get_group_release_version(
return release_version or None


class _CommitDataKwargs(TypedDict, total=False):
author: CommitAuthor
message: str
date_added: str


@region_silo_model
class Release(Model):
"""
Expand Down Expand Up @@ -658,7 +665,6 @@ def set_commits(self, commit_list):

# TODO(dcramer): this function could use some cleanup/refactoring as it's a bit unwieldy
from sentry.models.commit import Commit
from sentry.models.commitauthor import CommitAuthor
from sentry.models.group import Group, GroupStatus
from sentry.models.grouplink import GroupLink
from sentry.models.groupresolution import GroupResolution
Expand Down Expand Up @@ -752,7 +758,7 @@ def set_commits(self, commit_list):
else:
author = authors[author_email]

commit_data = {}
commit_data: _CommitDataKwargs = {}

# Update/set message and author if they are provided.
if author is not None:
Expand All @@ -768,14 +774,10 @@ def set_commits(self, commit_list):
key=data["id"],
defaults=commit_data,
)
if not created:
commit_data = {
key: value
for key, value in commit_data.items()
if getattr(commit, key) != value
}
if commit_data:
commit.update(**commit_data)
Comment on lines -771 to -778
Copy link
Member Author

Choose a reason for hiding this comment

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

this is ever-so-slightly slower in that it sometimes updates more things -- but greatly simplifies the types here (commit_data being reassigned to dict[str, object] here)

if not created and any(
getattr(commit, key) != value for key, value in commit_data.items()
):
commit.update(**commit_data)

if author is None:
author = commit.author
Expand Down
Loading