Skip to content

Commit 10ebd71

Browse files
authored
chore: cleanup unused ReleaseFileCache class (#94023)
The last usage of this class was removed here: https://github.com/getsentry/sentry/pull/59090/files#diff-5649208c3eac2367f8d3130a299d7c3cd3020aa4c1a21f1d4ea57b80de38ef34L439 with the last invocation of `read_artifact_index` with `use_cache=True`
1 parent 2f28a2c commit 10ebd71

File tree

5 files changed

+5
-124
lines changed

5 files changed

+5
-124
lines changed

src/sentry/api/endpoints/project_release_file_details.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ def download(releasefile):
8989
def download_from_archive(release, entry):
9090
archive_ident = entry["archive_ident"]
9191

92-
# Do not use ReleaseFileCache here, we view download as a singular event
9392
archive_file = ReleaseFile.objects.get(release_id=release.id, ident=archive_ident)
9493
archive_file_fp = archive_file.file.getfile()
9594
fp = ZipFile(archive_file_fp).open(entry["filename"])

src/sentry/bgtasks/clean_releasefilecache.py

Lines changed: 0 additions & 7 deletions
This file was deleted.

src/sentry/conf/server.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1380,10 +1380,6 @@ def SOCIAL_AUTH_DEFAULT_USERNAME() -> str:
13801380

13811381
BGTASKS: dict[str, BgTaskConfig] = {
13821382
"sentry.bgtasks.clean_dsymcache:clean_dsymcache": {"interval": 5 * 60, "roles": ["worker"]},
1383-
"sentry.bgtasks.clean_releasefilecache:clean_releasefilecache": {
1384-
"interval": 5 * 60,
1385-
"roles": ["worker"],
1386-
},
13871383
}
13881384

13891385
#######################

src/sentry/models/releasefile.py

Lines changed: 5 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
from __future__ import annotations
22

3-
import errno
43
import logging
5-
import os
64
import zipfile
75
from contextlib import contextmanager
86
from hashlib import sha1
@@ -12,10 +10,8 @@
1210
from urllib.parse import urlunsplit
1311

1412
import sentry_sdk
15-
from django.core.files.base import File as FileObj
1613
from django.db import models, router
1714

18-
from sentry import options
1915
from sentry.backup.scopes import RelocationScope
2016
from sentry.db.models import (
2117
BoundedBigIntegerField,
@@ -28,9 +24,8 @@
2824
from sentry.db.models.manager.base import BaseManager
2925
from sentry.models.distribution import Distribution
3026
from sentry.models.files.file import File
31-
from sentry.models.files.utils import clear_cached_files
3227
from sentry.models.release import Release
33-
from sentry.utils import json, metrics
28+
from sentry.utils import json
3429
from sentry.utils.db import atomic_transaction
3530
from sentry.utils.hashlib import sha1_text
3631
from sentry.utils.urls import urlsplit_best_effort
@@ -90,8 +85,6 @@ class ReleaseFile(Model):
9085
objects: ClassVar[BaseManager[Self]] = BaseManager() # The default manager.
9186
public_objects: ClassVar[PublicReleaseFileManager] = PublicReleaseFileManager()
9287

93-
cache: ClassVar[ReleaseFileCache]
94-
9588
class Meta:
9689
unique_together = (("release_id", "ident"),)
9790
indexes = (models.Index(fields=("release_id", "name")),)
@@ -152,48 +145,6 @@ def normalize(cls, url):
152145
return urls
153146

154147

155-
class ReleaseFileCache:
156-
@property
157-
def cache_path(self):
158-
return options.get("releasefile.cache-path")
159-
160-
def getfile(self, releasefile):
161-
cutoff = options.get("releasefile.cache-limit")
162-
file_size = releasefile.file.size
163-
if file_size < cutoff:
164-
metrics.distribution(
165-
"release_file.cache.get.size", file_size, tags={"cutoff": True}, unit="byte"
166-
)
167-
return releasefile.file.getfile()
168-
169-
file_id = str(releasefile.file.id)
170-
organization_id = str(releasefile.organization_id)
171-
file_path = os.path.join(self.cache_path, organization_id, file_id)
172-
173-
hit = True
174-
try:
175-
os.stat(file_path)
176-
except OSError as e:
177-
if e.errno != errno.ENOENT:
178-
raise
179-
releasefile.file.save_to(file_path)
180-
hit = False
181-
182-
metrics.distribution(
183-
"release_file.cache.get.size",
184-
file_size,
185-
tags={"hit": hit, "cutoff": False},
186-
unit="byte",
187-
)
188-
return FileObj(open(file_path, "rb"))
189-
190-
def clear_old_entries(self):
191-
clear_cached_files(self.cache_path)
192-
193-
194-
ReleaseFile.cache = ReleaseFileCache()
195-
196-
197148
class ReleaseArchive:
198149
"""Read-only view of uploaded ZIP-archive of release files"""
199150

@@ -288,17 +239,14 @@ def __init__(self, release: Release, dist: Distribution | None, **filter_args):
288239
self._ident = ReleaseFile.get_ident(ARTIFACT_INDEX_FILENAME, dist and dist.name)
289240
self._filter_args = filter_args # Extra constraints on artifact index release file
290241

291-
def readable_data(self, use_cache: bool) -> dict | None:
242+
def readable_data(self) -> dict | None:
292243
"""Simple read, no synchronization necessary"""
293244
try:
294245
releasefile = self._releasefile_qs()[0]
295246
except IndexError:
296247
return None
297248
else:
298-
if use_cache:
299-
fp = ReleaseFile.cache.getfile(releasefile)
300-
else:
301-
fp = releasefile.file.getfile()
249+
fp = releasefile.file.getfile()
302250
with fp:
303251
return json.load(fp)
304252

@@ -384,12 +332,10 @@ def _key_fields(self):
384332

385333

386334
@sentry_sdk.tracing.trace
387-
def read_artifact_index(
388-
release: Release, dist: Distribution | None, use_cache: bool = False, **filter_args
389-
) -> dict | None:
335+
def read_artifact_index(release: Release, dist: Distribution | None, **filter_args) -> dict | None:
390336
"""Get index data"""
391337
guard = _ArtifactIndexGuard(release, dist, **filter_args)
392-
return guard.readable_data(use_cache)
338+
return guard.readable_data()
393339

394340

395341
def _compute_sha1(archive: ReleaseArchive, url: str) -> str:

tests/sentry/models/test_releasefile.py

Lines changed: 0 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import errno
2-
import os
31
from datetime import datetime, timezone
42
from io import BytesIO
53
from threading import Thread
@@ -8,7 +6,6 @@
86

97
import pytest
108

11-
from sentry import options
129
from sentry.models.distribution import Distribution
1310
from sentry.models.files.file import File
1411
from sentry.models.releasefile import (
@@ -74,56 +71,6 @@ def test_count_artifacts(self):
7471
assert self.release.count_artifacts() == 5
7572

7673

77-
class ReleaseFileCacheTest(TestCase):
78-
def test_getfile_fs_cache(self):
79-
file_content = b"this is a test"
80-
81-
file = self.create_file(name="dummy.txt")
82-
file.putfile(BytesIO(file_content))
83-
release_file = self.create_release_file(file=file)
84-
85-
expected_path = os.path.join(
86-
options.get("releasefile.cache-path"),
87-
str(self.organization.id),
88-
str(file.id),
89-
)
90-
91-
# Set the threshold to zero to force caching on the file system
92-
options.set("releasefile.cache-limit", 0)
93-
with ReleaseFile.cache.getfile(release_file) as f:
94-
assert f.read() == file_content
95-
assert f.name == expected_path
96-
97-
# Check that the file was cached
98-
os.stat(expected_path)
99-
100-
def test_getfile_streaming(self):
101-
file_content = b"this is a test"
102-
103-
file = self.create_file(name="dummy.txt")
104-
file.putfile(BytesIO(file_content))
105-
release_file = self.create_release_file(file=file)
106-
107-
expected_path = os.path.join(
108-
options.get("releasefile.cache-path"),
109-
str(self.organization.id),
110-
str(file.id),
111-
)
112-
113-
# Set the threshold larger than the file size to force streaming
114-
options.set("releasefile.cache-limit", 1024)
115-
with ReleaseFile.cache.getfile(release_file) as f:
116-
assert f.read() == file_content
117-
118-
# Check that the file was not cached
119-
try:
120-
os.stat(expected_path)
121-
except OSError as e:
122-
assert e.errno == errno.ENOENT
123-
else:
124-
assert False, "file should not exist"
125-
126-
12774
class ReleaseArchiveTestCase(TestCase):
12875
def create_archive(self, fields, files, dist=None):
12976
manifest = dict(

0 commit comments

Comments
 (0)