Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 3 additions & 3 deletions src/sentry/models/release.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from sentry.db.models.indexes import IndexWithPostgresNameLimits
from sentry.db.models.manager.base import BaseManager
from sentry.models.artifactbundle import ArtifactBundle
from sentry.models.commit import Commit
from sentry.models.commitauthor import CommitAuthor
from sentry.models.releases.constants import (
DB_VERSION_LENGTH,
Expand All @@ -41,7 +42,6 @@
from sentry.models.releases.exceptions import UnsafeReleaseDeletion
from sentry.models.releases.release_project import ReleaseProject
from sentry.models.releases.util import ReleaseQuerySet, SemverFilter, SemverVersion
from sentry.releases.commits import get_or_create_commit
from sentry.utils import metrics
from sentry.utils.cache import cache
from sentry.utils.db import atomic_transaction
Expand Down Expand Up @@ -618,8 +618,8 @@ def set_refs(self, refs, user_id, fetch=False):
for ref in refs:
repo = repos_by_name[ref["repository"]]

commit = get_or_create_commit(
organization=self.organization, repo_id=repo.id, key=ref["commit"]
commit = Commit.objects.get_or_create(
organization_id=self.organization_id, repository_id=repo.id, key=ref["commit"]
)[0]
# update head commit for repo/release if exists
ReleaseHeadCommit.objects.create_or_update(
Expand Down
14 changes: 5 additions & 9 deletions src/sentry/models/releases/set_commits.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from sentry.db.postgres.transactions import in_test_hide_transaction_boundary
from sentry.locks import locks
from sentry.models.activity import Activity
from sentry.models.commit import Commit
from sentry.models.commitauthor import CommitAuthor
from sentry.models.commitfilechange import CommitFileChange
from sentry.models.grouphistory import GroupHistoryStatus, record_group_history
Expand All @@ -34,7 +35,6 @@
from sentry.models.releaseheadcommit import ReleaseHeadCommit
from sentry.models.repository import Repository
from sentry.plugins.providers.repository import RepositoryProvider
from sentry.releases.commits import get_or_create_commit


class _CommitDataKwargs(TypedDict, total=False):
Expand Down Expand Up @@ -123,15 +123,11 @@ def set_commit(idx, data, release):
if "timestamp" in data:
commit_data["date_added"] = data["timestamp"]

commit, new_commit, created = get_or_create_commit(
organization=release.organization,
repo_id=repo.id,
commit, created = Commit.objects.get_or_create(
organization_id=release.organization_id,
repository_id=repo.id,
key=data["id"],
message=commit_data.get("message"),
author=commit_data.get("author"),
# XXX: This code was in place before and passes either a string or datetime, but
# works ok. Just skipping the type checking
date_added=commit_data.get("date_added"), # type: ignore[arg-type]
defaults=commit_data,
)
if not created and any(getattr(commit, key) != value for key, value in commit_data.items()):
commit.update(**commit_data)
Expand Down
36 changes: 0 additions & 36 deletions src/sentry/releases/commits.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,39 +65,3 @@ def create_commit(
)
new_commit = _dual_write_commit(old_commit)
return old_commit, new_commit


def get_or_create_commit(
organization: Organization,
repo_id: int,
key: str,
message: str | None = None,
author: CommitAuthor | None = None,
date_added: datetime | None = None,
) -> tuple[OldCommit, Commit, bool]:
"""
Gets or creates a commit with dual write support.
"""
defaults = {
"author": author,
"message": message,
}
if date_added is not None:
defaults["date_added"] = date_added # type: ignore[assignment]

with atomic_transaction(
using=(
router.db_for_write(OldCommit),
router.db_for_write(Commit),
)
):
old_commit, created = OldCommit.objects.get_or_create(
organization_id=organization.id,
repository_id=repo_id,
key=key,
defaults=defaults,
)

new_commit = _dual_write_commit(old_commit)

return old_commit, new_commit, created
89 changes: 1 addition & 88 deletions tests/sentry/releases/test_commits.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from sentry.models.commit import Commit as OldCommit
from sentry.models.commitauthor import CommitAuthor
from sentry.models.repository import Repository
from sentry.releases.commits import create_commit, get_or_create_commit
from sentry.releases.commits import create_commit
from sentry.releases.models import Commit
from sentry.testutils.cases import TestCase

Expand Down Expand Up @@ -143,90 +143,3 @@ def test_create_commit_transaction_atomicity(self):
)
assert not OldCommit.objects.filter(key="test_atomicity_key").exists()
assert not Commit.objects.filter(key="test_atomicity_key").exists()


class GetOrCreateCommitDualWriteTest(TestCase):
def setUp(self):
super().setUp()
self.repo = Repository.objects.create(
name="test-repo",
organization_id=self.organization.id,
)
self.author = CommitAuthor.objects.create(
organization_id=self.organization.id,
email="test@example.com",
name="Test Author",
)

def test_get_or_create_commit_creates_new(self):
"""Test that get_or_create creates a new commit when it doesn't exist"""
old_commit, new_commit, created = get_or_create_commit(
organization=self.organization,
repo_id=self.repo.id,
key="new123",
message="New commit message",
author=self.author,
)

assert created is True
assert old_commit.key == "new123"
assert old_commit.message == "New commit message"
assert old_commit.author == self.author

assert new_commit is not None
assert new_commit.id == old_commit.id
assert new_commit.key == "new123"
assert new_commit.message == "New commit message"
assert new_commit.author == self.author

def test_get_or_create_commit_gets_existing(self):
"""Test that get_or_create returns existing commit when it exists"""
existing_old, existing_new = create_commit(
organization=self.organization,
repo_id=self.repo.id,
key="existing456",
message="Existing commit",
author=self.author,
)
assert existing_new is not None
old_commit, new_commit, created = get_or_create_commit(
organization=self.organization,
repo_id=self.repo.id,
key="existing456",
message="This should not be used",
author=None,
)
assert created is False
assert old_commit.id == existing_old.id
assert old_commit.key == "existing456"
assert old_commit.message == "Existing commit"
assert old_commit.author == self.author
assert new_commit is not None
assert new_commit.id == existing_new.id

def test_get_or_create_commit_backfills_to_new_table(self):
"""Test that get_or_create backfills to new table if commit exists only in old table"""
old_only_commit = OldCommit.objects.create(
organization_id=self.organization.id,
repository_id=self.repo.id,
key="old_only789",
message="Old table only",
author=self.author,
)
assert not Commit.objects.filter(id=old_only_commit.id).exists()

old_commit, new_commit, created = get_or_create_commit(
organization=self.organization,
repo_id=self.repo.id,
key="old_only789",
message="Should not be used",
)

assert created is False
assert old_commit.id == old_only_commit.id
assert old_commit.message == "Old table only"
assert new_commit is not None
assert new_commit.id == old_only_commit.id
assert new_commit.key == "old_only789"
assert new_commit.message == "Old table only"
assert new_commit.author == self.author
Loading