-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat(iswf): Adds Issue label, org label coarse data model #114925
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
base: master
Are you sure you want to change the base?
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,3 @@ | ||
| from .service import issue_label_service | ||
|
|
||
| __all__ = ("issue_label_service",) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| from __future__ import annotations | ||
|
|
||
| from django.core.cache import cache | ||
|
|
||
| from sentry import options | ||
| from sentry.models.issuelabel import IssueLabel | ||
| from sentry.models.organizationlabel import OrganizationLabel | ||
|
|
||
|
|
||
| class IssueLabelCache: | ||
| """ | ||
| Caches IssueLabel query results keyed by issue (group) ID. | ||
|
|
||
| Uses Django's cache backend (Redis in production). TTL is controlled | ||
| by the ``issues.issue-label-cache-ttl`` option (default 10 minutes). | ||
| """ | ||
|
|
||
| KEY_PREFIX = "issuelabel:g" | ||
|
|
||
| @classmethod | ||
| def _make_key(cls, group_id: int) -> str: | ||
| return f"{cls.KEY_PREFIX}:{group_id}" | ||
|
|
||
| @classmethod | ||
| def get(cls, group_id: int) -> list[IssueLabel] | None: | ||
| """Return cached label list for the given issue, or None on cache miss.""" | ||
| return cache.get(cls._make_key(group_id)) | ||
|
|
||
| @classmethod | ||
| def set(cls, group_id: int, values: list[IssueLabel]) -> None: | ||
| """Store label list in cache for the given issue.""" | ||
| cache.set(cls._make_key(group_id), values, options.get("issues.issue-label-cache-ttl")) | ||
|
|
||
| @classmethod | ||
| def invalidate(cls, group_id: int) -> None: | ||
| """Remove cached entry for the given issue.""" | ||
| cache.delete(cls._make_key(group_id)) | ||
|
|
||
|
|
||
| class OrganizationLabelCache: | ||
| """ | ||
| Caches OrganizationLabel query results keyed by organization ID. | ||
|
|
||
| Uses Django's cache backend (Redis in production). TTL is controlled | ||
| by the ``issues.org-label-cache-ttl`` option (default 10 minutes). | ||
| """ | ||
|
|
||
| KEY_PREFIX = "orglabel:o" | ||
|
|
||
| @classmethod | ||
| def _make_key(cls, organization_id: int) -> str: | ||
| return f"{cls.KEY_PREFIX}:{organization_id}" | ||
|
|
||
| @classmethod | ||
| def get(cls, organization_id: int) -> list[OrganizationLabel] | None: | ||
| """Return cached label list for the given organization, or None on cache miss.""" | ||
| return cache.get(cls._make_key(organization_id)) | ||
|
|
||
| @classmethod | ||
| def set(cls, organization_id: int, values: list[OrganizationLabel]) -> None: | ||
| """Store label list in cache for the given organization.""" | ||
| cache.set(cls._make_key(organization_id), values, options.get("issues.org-label-cache-ttl")) | ||
|
|
||
| @classmethod | ||
| def invalidate(cls, organization_id: int) -> None: | ||
| """Remove cached entry for the given organization.""" | ||
| cache.delete(cls._make_key(organization_id)) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| from sentry.issues.services.issue_label.cache import IssueLabelCache, OrganizationLabelCache | ||
| from sentry.models.issuelabel import IssueLabel | ||
| from sentry.models.organizationlabel import OrganizationLabel | ||
|
|
||
|
|
||
| class IssueLabelService: | ||
| """ | ||
| Service layer for creating, updating, and deleting IssueLabel records. | ||
|
|
||
| All mutations invalidate the cache so subsequent reads reflect the change. | ||
| """ | ||
|
|
||
| def __init__(self) -> None: | ||
| self.cache = IssueLabelCache | ||
| self.org_label_cache = OrganizationLabelCache | ||
|
|
||
| def get_by_group_id(self, group_id: int) -> list[IssueLabel]: | ||
| """Return all IssueLabels for the given issue, using cache.""" | ||
| cached = self.cache.get(group_id) | ||
| if cached is not None: | ||
| return cached | ||
|
|
||
| labels = list(IssueLabel.objects.filter(group_id=group_id).select_related("label")) | ||
| self.cache.set(group_id, labels) | ||
| return labels | ||
|
|
||
| def create(self, *, group_id: int, label_id: int, label_value: str) -> IssueLabel: | ||
| """Create a new IssueLabel and invalidate the cache for its issue.""" | ||
| issue_label = IssueLabel.objects.create( | ||
| group_id=group_id, | ||
| label_id=label_id, | ||
| label_value=label_value, | ||
|
Check warning on line 32 in src/sentry/issues/services/issue_label/service.py
|
||
| ) | ||
| self.cache.invalidate(group_id) | ||
| return issue_label | ||
|
|
||
| def update( | ||
| self, | ||
| *, | ||
| issue_label_id: int, | ||
| label_id: int | None = None, | ||
| label_value: str | None = None, | ||
| ) -> IssueLabel: | ||
| """ | ||
| Update an IssueLabel by id. | ||
|
|
||
| Invalidates the cache for the owning issue. | ||
|
Check warning on line 47 in src/sentry/issues/services/issue_label/service.py
|
||
|
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. IssueLabel.update() raises DoesNotExist on missing/deleted record
VerificationReviewed the service module in the hunk. Also found at 2 additional locations
Identified by Warden |
||
| """ | ||
| issue_label = IssueLabel.objects.get(id=issue_label_id) | ||
|
|
||
| update_fields: list[str] = [] | ||
| if label_id is not None: | ||
| issue_label.label_id = label_id | ||
| update_fields.append("label_id") | ||
| if label_value is not None: | ||
| issue_label.label_value = label_value | ||
| update_fields.append("label_value") | ||
|
|
||
| if update_fields: | ||
| issue_label.save(update_fields=update_fields) | ||
|
|
||
| self.cache.invalidate(issue_label.group_id) | ||
| return issue_label | ||
|
|
||
| def delete(self, *, issue_label_id: int) -> None: | ||
|
Check warning on line 65 in src/sentry/issues/services/issue_label/service.py
|
||
| """Delete an IssueLabel by id and invalidate the cache for its issue.""" | ||
| issue_label = IssueLabel.objects.get(id=issue_label_id) | ||
| group_id = issue_label.group_id | ||
| issue_label.delete() | ||
| self.cache.invalidate(group_id) | ||
|
|
||
| # -- OrganizationLabel operations -- | ||
|
|
||
| def get_org_labels(self, organization_id: int) -> list[OrganizationLabel]: | ||
| """Return all OrganizationLabels for the given organization, using cache.""" | ||
| cached = self.org_label_cache.get(organization_id) | ||
| if cached is not None: | ||
| return cached | ||
|
|
||
| labels = list( | ||
| OrganizationLabel.objects.filter(organization_id=organization_id).order_by("label_name") | ||
| ) | ||
| self.org_label_cache.set(organization_id, labels) | ||
| return labels | ||
|
|
||
| def create_org_label(self, *, organization_id: int, label_name: str) -> OrganizationLabel: | ||
| """Create a new OrganizationLabel and invalidate the cache for the organization.""" | ||
| org_label = OrganizationLabel.objects.create( | ||
| organization_id=organization_id, | ||
| label_name=label_name, | ||
| ) | ||
| self.org_label_cache.invalidate(organization_id) | ||
| return org_label | ||
|
|
||
| def update_org_label(self, *, org_label_id: int, label_name: str) -> OrganizationLabel: | ||
| """Update an OrganizationLabel's name and invalidate the cache for its organization.""" | ||
| org_label = OrganizationLabel.objects.get(id=org_label_id) | ||
| org_label.label_name = label_name | ||
| org_label.save(update_fields=["label_name"]) | ||
|
Check warning on line 99 in src/sentry/issues/services/issue_label/service.py
|
||
| self.org_label_cache.invalidate(org_label.organization_id) | ||
| return org_label | ||
|
|
||
| def delete_org_label(self, *, org_label_id: int) -> None: | ||
| """Delete an OrganizationLabel and invalidate the cache for its organization.""" | ||
| org_label = OrganizationLabel.objects.get(id=org_label_id) | ||
| organization_id = org_label.organization_id | ||
| org_label.delete() | ||
| self.org_label_cache.invalidate(organization_id) | ||
|
|
||
|
|
||
| issue_label_service = IssueLabelService() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| # Generated by Django 5.2.12 on 2026-05-04 23:37 | ||
|
|
||
| import django.db.models.deletion | ||
| import sentry.db.models.fields.bounded | ||
| import sentry.db.models.fields.foreignkey | ||
| from django.db import migrations, models | ||
|
|
||
| from sentry.new_migrations.migrations import CheckedMigration | ||
|
|
||
|
|
||
| class Migration(CheckedMigration): | ||
| # This flag is used to mark that a migration shouldn't be automatically run in production. | ||
| # This should only be used for operations where it's safe to run the migration after your | ||
| # code has deployed. So this should not be used for most operations that alter the schema | ||
| # of a table. | ||
| # Here are some things that make sense to mark as post deployment: | ||
| # - Large data migrations. Typically we want these to be run manually so that they can be | ||
| # monitored and not block the deploy for a long period of time while they run. | ||
| # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to | ||
| # run this outside deployments so that we don't block them. Note that while adding an index | ||
| # is a schema change, it's completely safe to run the operation after the code has deployed. | ||
| # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment | ||
|
|
||
| is_post_deployment = False | ||
|
|
||
| dependencies = [ | ||
| ("sentry", "1080_backfill_deprecated_dashboard_widget_display_types"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.CreateModel( | ||
| name="OrganizationLabel", | ||
| fields=[ | ||
| ( | ||
| "id", | ||
| sentry.db.models.fields.bounded.BoundedBigAutoField( | ||
| primary_key=True, serialize=False | ||
| ), | ||
| ), | ||
| ("label_name", models.CharField(max_length=255)), | ||
| ( | ||
| "organization", | ||
| sentry.db.models.fields.foreignkey.FlexibleForeignKey( | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| related_name="organizationlabel_set", | ||
| to="sentry.organization", | ||
| ), | ||
| ), | ||
| ], | ||
| options={ | ||
| "db_table": "sentry_organizationlabel", | ||
| "unique_together": {("organization", "label_name")}, | ||
| }, | ||
| ), | ||
| migrations.CreateModel( | ||
| name="IssueLabel", | ||
| fields=[ | ||
| ( | ||
| "id", | ||
| sentry.db.models.fields.bounded.BoundedBigAutoField( | ||
| primary_key=True, serialize=False | ||
| ), | ||
| ), | ||
| ("label_value", models.CharField(max_length=255)), | ||
| ( | ||
| "group", | ||
| sentry.db.models.fields.foreignkey.FlexibleForeignKey( | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| related_name="issuelabel_set", | ||
| to="sentry.group", | ||
| ), | ||
| ), | ||
| ( | ||
| "label", | ||
| sentry.db.models.fields.foreignkey.FlexibleForeignKey( | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| related_name="issuelabel_set", | ||
| to="sentry.organizationlabel", | ||
| ), | ||
| ), | ||
| ], | ||
| options={ | ||
| "db_table": "sentry_issuelabel", | ||
| "indexes": [ | ||
| models.Index(fields=["group", "label"], name="sentry_issu_group_i_0f746e_idx") | ||
| ], | ||
| }, | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| from django.db import models | ||
|
|
||
| from sentry.backup.scopes import RelocationScope | ||
| from sentry.db.models import FlexibleForeignKey, Model, cell_silo_model, sane_repr | ||
|
|
||
|
|
||
| @cell_silo_model | ||
| class IssueLabel(Model): | ||
| """ | ||
| Attaches an organization-defined label with a value to an issue. | ||
| """ | ||
|
|
||
| __relocation_scope__ = RelocationScope.Excluded | ||
|
|
||
| group = FlexibleForeignKey("sentry.Group", related_name="issuelabel_set") | ||
| label = FlexibleForeignKey("sentry.OrganizationLabel", related_name="issuelabel_set") | ||
| label_value = models.CharField(max_length=255) | ||
|
|
||
| class Meta: | ||
| app_label = "sentry" | ||
| db_table = "sentry_issuelabel" | ||
| indexes = [ | ||
| models.Index(fields=["group", "label"]), | ||
| ] | ||
|
|
||
| __repr__ = sane_repr("group_id", "label_id", "label_value") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| from django.db import models | ||
|
|
||
| from sentry.backup.scopes import RelocationScope | ||
| from sentry.db.models import FlexibleForeignKey, Model, cell_silo_model, sane_repr | ||
|
|
||
|
|
||
| @cell_silo_model | ||
| class OrganizationLabel(Model): | ||
| """ | ||
| Defines a label name scoped to an organization. | ||
|
|
||
| IssueLabel references this model to attach labels to individual issues. | ||
| """ | ||
|
|
||
| __relocation_scope__ = RelocationScope.Excluded | ||
|
|
||
| organization = FlexibleForeignKey("sentry.Organization", related_name="organizationlabel_set") | ||
| label_name = models.CharField(max_length=255) | ||
|
|
||
| class Meta: | ||
| app_label = "sentry" | ||
| db_table = "sentry_organizationlabel" | ||
| unique_together = (("organization", "label_name"),) | ||
|
|
||
| __repr__ = sane_repr("organization_id", "label_name") |
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.
create() and create_org_label() do not handle unique constraint violations
IssueLabel.objects.create()andOrganizationLabel.objects.create()are called without handlingIntegrityError. If unique constraints exist (likely for(group_id, label_id)on IssueLabel and(organization_id, label_name)on OrganizationLabel based on typical label data models), duplicate creation requests will raise an unhandledIntegrityErrorresulting in a 500. This is Check 7 (Database Constraint Violations).Verification
Read the create() and create_org_label() methods in the hunk. Cannot confirm exact unique constraints without reading src/sentry/models/issuelabel.py and src/sentry/models/organizationlabel.py (listed as other files in PR but not provided), but label models almost universally enforce uniqueness on (scope, name). Reporting at medium confidence since the constraint definitions aren't visible — the pattern (unguarded create on a label model) is a well-known precedent for IntegrityError issues.
Identified by Warden
sentry-backend-bugs·GAX-S46