-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat(difs): Add Objectstore columns to ProjectDebugFile #117500
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| from django.db import migrations, models | ||
|
|
||
| import sentry.db.models.fields.bounded | ||
| import sentry.db.models.fields.foreignkey | ||
|
|
||
| from sentry.new_migrations.migrations import CheckedMigration | ||
|
|
||
|
|
||
| class Migration(CheckedMigration): | ||
| is_post_deployment = False | ||
|
|
||
| dependencies = [ | ||
| ("sentry", "1114_extend_repository_url_length"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AlterField( | ||
| model_name="projectdebugfile", | ||
| name="file", | ||
| field=sentry.db.models.fields.foreignkey.FlexibleForeignKey( | ||
| null=True, on_delete=models.PROTECT, to="sentry.file" | ||
| ), | ||
| ), | ||
| migrations.AddField( | ||
| model_name="projectdebugfile", | ||
| name="storage_path", | ||
| field=models.TextField(null=True), | ||
| ), | ||
| migrations.AddField( | ||
| model_name="projectdebugfile", | ||
| name="content_type", | ||
| field=models.TextField(null=True), | ||
| ), | ||
| migrations.AddField( | ||
| model_name="projectdebugfile", | ||
| name="file_size", | ||
| field=sentry.db.models.fields.bounded.BoundedBigIntegerField(null=True), | ||
| ), | ||
| migrations.AddField( | ||
| model_name="projectdebugfile", | ||
| name="date_created", | ||
| field=models.DateTimeField(null=True), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,7 +119,8 @@ def find_by_debug_ids( | |
| class ProjectDebugFile(Model): | ||
| __relocation_scope__ = RelocationScope.Excluded | ||
|
|
||
| file = FlexibleForeignKey("sentry.File", on_delete=models.PROTECT) | ||
| # When the migration to Objectstore is complete, this can be removed. | ||
| file = FlexibleForeignKey("sentry.File", null=True, on_delete=models.PROTECT) | ||
| checksum = models.CharField(max_length=40, null=True, db_index=True) | ||
| object_name = models.TextField() | ||
| cpu_name = models.CharField(max_length=40) | ||
|
|
@@ -129,6 +130,16 @@ class ProjectDebugFile(Model): | |
| data = LegacyTextJSONField(default=dict, null=True) | ||
| date_accessed = models.DateTimeField(default=timezone.now, db_default=Now()) | ||
|
|
||
| # The following fields are present if and only if the file is stored in Objectstore. | ||
| # Key of the file in Objectstore. | ||
| storage_path = models.TextField(null=True) | ||
| # Mirrors `file.headers["Content-Type"]` for files stored in Objectstore. | ||
| content_type = models.TextField(null=True) | ||
| # Mirrors `file.size` for files stored in Objectstore. | ||
| file_size = BoundedBigIntegerField(null=True) | ||
| # Mirrors `file.timestamp` for files stored in Objectstore. | ||
| date_created = models.DateTimeField(null=True) | ||
|
|
||
| objects: ClassVar[ProjectDebugFileManager] = ProjectDebugFileManager() | ||
|
|
||
| difcache: ClassVar[DIFCache] | ||
|
|
@@ -145,6 +156,7 @@ class Meta: | |
|
|
||
| @property | ||
| def file_format(self) -> str: | ||
| assert self.file is not None | ||
| ct = self.file.headers.get("Content-Type", "unknown").lower() | ||
| return KNOWN_DIF_FORMATS.get(ct, "unknown") | ||
|
|
||
|
|
@@ -201,7 +213,8 @@ def delete(self, *args: Any, **kwargs: Any) -> tuple[int, dict[str, int]]: | |
| # row behind, but no surviving ProjectDebugFile should point to a deleted | ||
| # File. | ||
| try: | ||
| self.file.delete() | ||
| if self.file is not None: | ||
| self.file.delete() | ||
| except ProtectedError: | ||
| pass | ||
|
|
||
|
|
@@ -224,6 +237,7 @@ def clean_redundant_difs(project: Project, debug_id: str) -> None: | |
| uuidmap_seen = False | ||
| il2cpp_seen = False | ||
| for i, dif in enumerate(difs): | ||
| assert dif.file is not None | ||
| mime_type = dif.file.headers.get("Content-Type") | ||
| if mime_type == DIF_MIMETYPES["bcsymbolmap"]: | ||
| if not bcsymbolmap_seen: | ||
|
Comment on lines
237
to
243
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: The Suggested FixModify Prompt for AI AgentDid we get this right? 👍 / 👎 to inform future reviews. |
||
|
|
@@ -733,6 +747,7 @@ def fetch_difs( | |
| except OSError as e: | ||
| if e.errno != errno.ENOENT: | ||
| raise | ||
| assert dif.file is not None | ||
| dif.file.save_to(dif_path) | ||
| rv[debug_id] = dif_path | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Serializing a
DebugFilewith aNonefile attribute (for Objectstore-backed DIFs) raises anAssertionErrororAttributeErrorbecause the serializer does not handle the null case.Severity: CRITICAL
Suggested Fix
Update
DebugFileSerializerto handle cases whereobj.fileisNone. Useobj.content_typeand other direct attributes from theDebugFilemodel for Objectstore-backed DIFs instead of accessing them through the now-nullablefilerelation. Remove theassert self.file is not Nonefrom thefile_formatproperty and add logic to handle theNonecase.Prompt for AI Agent
Also affects:
src/sentry/models/debugfile.py:130~136Did we get this right? 👍 / 👎 to inform future reviews.