Skip to content

Commit 03b3777

Browse files
wedamijashashjar
authored andcommitted
chore(commits): Remove get_or_create_commit dual write function (#102259)
We're no longer planning on using the dual written tables, so removing these functions <!-- Describe your PR here. -->
1 parent ca1dbd3 commit 03b3777

File tree

4 files changed

+9
-136
lines changed

4 files changed

+9
-136
lines changed

src/sentry/models/release.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
from sentry.db.models.indexes import IndexWithPostgresNameLimits
3333
from sentry.db.models.manager.base import BaseManager
3434
from sentry.models.artifactbundle import ArtifactBundle
35+
from sentry.models.commit import Commit
3536
from sentry.models.commitauthor import CommitAuthor
3637
from sentry.models.releases.constants import (
3738
DB_VERSION_LENGTH,
@@ -41,7 +42,6 @@
4142
from sentry.models.releases.exceptions import UnsafeReleaseDeletion
4243
from sentry.models.releases.release_project import ReleaseProject
4344
from sentry.models.releases.util import ReleaseQuerySet, SemverFilter, SemverVersion
44-
from sentry.releases.commits import get_or_create_commit
4545
from sentry.utils import metrics
4646
from sentry.utils.cache import cache
4747
from sentry.utils.db import atomic_transaction
@@ -618,8 +618,8 @@ def set_refs(self, refs, user_id, fetch=False):
618618
for ref in refs:
619619
repo = repos_by_name[ref["repository"]]
620620

621-
commit = get_or_create_commit(
622-
organization=self.organization, repo_id=repo.id, key=ref["commit"]
621+
commit = Commit.objects.get_or_create(
622+
organization_id=self.organization_id, repository_id=repo.id, key=ref["commit"]
623623
)[0]
624624
# update head commit for repo/release if exists
625625
ReleaseHeadCommit.objects.create_or_update(

src/sentry/models/releases/set_commits.py

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from sentry.db.postgres.transactions import in_test_hide_transaction_boundary
1212
from sentry.locks import locks
1313
from sentry.models.activity import Activity
14+
from sentry.models.commit import Commit
1415
from sentry.models.commitauthor import CommitAuthor
1516
from sentry.models.commitfilechange import CommitFileChange
1617
from sentry.models.grouphistory import GroupHistoryStatus, record_group_history
@@ -34,7 +35,6 @@
3435
from sentry.models.releaseheadcommit import ReleaseHeadCommit
3536
from sentry.models.repository import Repository
3637
from sentry.plugins.providers.repository import RepositoryProvider
37-
from sentry.releases.commits import get_or_create_commit
3838

3939

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

126-
commit, new_commit, created = get_or_create_commit(
127-
organization=release.organization,
128-
repo_id=repo.id,
126+
commit, created = Commit.objects.get_or_create(
127+
organization_id=release.organization_id,
128+
repository_id=repo.id,
129129
key=data["id"],
130-
message=commit_data.get("message"),
131-
author=commit_data.get("author"),
132-
# XXX: This code was in place before and passes either a string or datetime, but
133-
# works ok. Just skipping the type checking
134-
date_added=commit_data.get("date_added"), # type: ignore[arg-type]
130+
defaults=commit_data,
135131
)
136132
if not created and any(getattr(commit, key) != value for key, value in commit_data.items()):
137133
commit.update(**commit_data)

src/sentry/releases/commits.py

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -65,39 +65,3 @@ def create_commit(
6565
)
6666
new_commit = _dual_write_commit(old_commit)
6767
return old_commit, new_commit
68-
69-
70-
def get_or_create_commit(
71-
organization: Organization,
72-
repo_id: int,
73-
key: str,
74-
message: str | None = None,
75-
author: CommitAuthor | None = None,
76-
date_added: datetime | None = None,
77-
) -> tuple[OldCommit, Commit, bool]:
78-
"""
79-
Gets or creates a commit with dual write support.
80-
"""
81-
defaults = {
82-
"author": author,
83-
"message": message,
84-
}
85-
if date_added is not None:
86-
defaults["date_added"] = date_added # type: ignore[assignment]
87-
88-
with atomic_transaction(
89-
using=(
90-
router.db_for_write(OldCommit),
91-
router.db_for_write(Commit),
92-
)
93-
):
94-
old_commit, created = OldCommit.objects.get_or_create(
95-
organization_id=organization.id,
96-
repository_id=repo_id,
97-
key=key,
98-
defaults=defaults,
99-
)
100-
101-
new_commit = _dual_write_commit(old_commit)
102-
103-
return old_commit, new_commit, created

tests/sentry/releases/test_commits.py

Lines changed: 1 addition & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from sentry.models.commit import Commit as OldCommit
88
from sentry.models.commitauthor import CommitAuthor
99
from sentry.models.repository import Repository
10-
from sentry.releases.commits import create_commit, get_or_create_commit
10+
from sentry.releases.commits import create_commit
1111
from sentry.releases.models import Commit
1212
from sentry.testutils.cases import TestCase
1313

@@ -143,90 +143,3 @@ def test_create_commit_transaction_atomicity(self):
143143
)
144144
assert not OldCommit.objects.filter(key="test_atomicity_key").exists()
145145
assert not Commit.objects.filter(key="test_atomicity_key").exists()
146-
147-
148-
class GetOrCreateCommitDualWriteTest(TestCase):
149-
def setUp(self):
150-
super().setUp()
151-
self.repo = Repository.objects.create(
152-
name="test-repo",
153-
organization_id=self.organization.id,
154-
)
155-
self.author = CommitAuthor.objects.create(
156-
organization_id=self.organization.id,
157-
158-
name="Test Author",
159-
)
160-
161-
def test_get_or_create_commit_creates_new(self):
162-
"""Test that get_or_create creates a new commit when it doesn't exist"""
163-
old_commit, new_commit, created = get_or_create_commit(
164-
organization=self.organization,
165-
repo_id=self.repo.id,
166-
key="new123",
167-
message="New commit message",
168-
author=self.author,
169-
)
170-
171-
assert created is True
172-
assert old_commit.key == "new123"
173-
assert old_commit.message == "New commit message"
174-
assert old_commit.author == self.author
175-
176-
assert new_commit is not None
177-
assert new_commit.id == old_commit.id
178-
assert new_commit.key == "new123"
179-
assert new_commit.message == "New commit message"
180-
assert new_commit.author == self.author
181-
182-
def test_get_or_create_commit_gets_existing(self):
183-
"""Test that get_or_create returns existing commit when it exists"""
184-
existing_old, existing_new = create_commit(
185-
organization=self.organization,
186-
repo_id=self.repo.id,
187-
key="existing456",
188-
message="Existing commit",
189-
author=self.author,
190-
)
191-
assert existing_new is not None
192-
old_commit, new_commit, created = get_or_create_commit(
193-
organization=self.organization,
194-
repo_id=self.repo.id,
195-
key="existing456",
196-
message="This should not be used",
197-
author=None,
198-
)
199-
assert created is False
200-
assert old_commit.id == existing_old.id
201-
assert old_commit.key == "existing456"
202-
assert old_commit.message == "Existing commit"
203-
assert old_commit.author == self.author
204-
assert new_commit is not None
205-
assert new_commit.id == existing_new.id
206-
207-
def test_get_or_create_commit_backfills_to_new_table(self):
208-
"""Test that get_or_create backfills to new table if commit exists only in old table"""
209-
old_only_commit = OldCommit.objects.create(
210-
organization_id=self.organization.id,
211-
repository_id=self.repo.id,
212-
key="old_only789",
213-
message="Old table only",
214-
author=self.author,
215-
)
216-
assert not Commit.objects.filter(id=old_only_commit.id).exists()
217-
218-
old_commit, new_commit, created = get_or_create_commit(
219-
organization=self.organization,
220-
repo_id=self.repo.id,
221-
key="old_only789",
222-
message="Should not be used",
223-
)
224-
225-
assert created is False
226-
assert old_commit.id == old_only_commit.id
227-
assert old_commit.message == "Old table only"
228-
assert new_commit is not None
229-
assert new_commit.id == old_only_commit.id
230-
assert new_commit.key == "old_only789"
231-
assert new_commit.message == "Old table only"
232-
assert new_commit.author == self.author

0 commit comments

Comments
 (0)