From 6a39d4d606fc7fc2d64573b7eba61125bb2d3ec3 Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Tue, 27 Oct 2026 13:56:55 -0400 Subject: [PATCH 1/7] Label incident action items and add a historical backfill --- README.md | 16 ++ scripts/backfill_incident_action_labels.py | 87 ++++++ src/firetower/incidents/services.py | 37 +++ .../tests/test_action_item_labels.py | 249 ++++++++++++++++++ src/firetower/integrations/services/linear.py | 85 +++++- .../integrations/tests/test_linear_service.py | 109 ++++++++ 6 files changed, 574 insertions(+), 9 deletions(-) create mode 100644 scripts/backfill_incident_action_labels.py create mode 100644 src/firetower/incidents/tests/test_action_item_labels.py diff --git a/README.md b/README.md index 2c3a73b2..42dbb568 100644 --- a/README.md +++ b/README.md @@ -127,6 +127,22 @@ frontend/ # React frontend sdk/ # Python SDK ``` +## Incident Action Labels + +Action item sync adds the existing workspace-wide `incident-action` label to incident sub-issues, including pre-existing issues adopted as follow-ups. Other labels are preserved. + +To backfill historical action items, use the script with the intended environment's Firetower configuration. It reads current sub-issues for every stored incident parent, including archived children, without changing parent relationships, SLAs, or cached action items. + +```sh +# Preview label additions without updating issues +uv run python scripts/backfill_incident_action_labels.py + +# Apply label additions after reviewing the preview +uv run python scripts/backfill_incident_action_labels.py --apply +``` + +The backfill is safe to rerun and exits nonzero on failed lookups or label updates. It does not create the workspace label if it is missing. + ## Scheduled Tasks Scheduled tasks are stored as database objects and are managed via migrations. diff --git a/scripts/backfill_incident_action_labels.py b/scripts/backfill_incident_action_labels.py new file mode 100644 index 00000000..d4c8a99e --- /dev/null +++ b/scripts/backfill_incident_action_labels.py @@ -0,0 +1,87 @@ +"""Backfill incident-action labels without changing issues' other fields. + +Preview: uv run python scripts/backfill_incident_action_labels.py +Apply: uv run python scripts/backfill_incident_action_labels.py --apply +""" + +import argparse +import logging +import os + +import django +from django.conf import settings + +logger = logging.getLogger(__name__) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--apply", action="store_true", help="Apply label additions") + args = parser.parse_args(argv) + + os.environ.setdefault("DJANGO_SETTINGS_MODULE", "firetower.settings") + django.setup() + + from firetower.incidents.models import Incident # noqa: PLC0415 + from firetower.integrations.services.linear import ( # noqa: PLC0415 + INCIDENT_ACTION_LABEL_NAME, + LinearService, + ) + + if not settings.LINEAR: + parser.error("Linear integration is not configured") + + service = LinearService() + label_id = service.get_issue_label_id(INCIDENT_ACTION_LABEL_NAME) + if not label_id: + logger.error("Workspace label %s is unavailable", INCIDENT_ACTION_LABEL_NAME) + return 1 + + incidents = Incident.objects.exclude(linear_parent_issue_id__isnull=True).exclude( + linear_parent_issue_id="" + ) + seen: set[str] = set() + missing = added = failed = 0 + for incident in incidents.iterator(): + children = service.get_child_issues( + incident.linear_parent_issue_id, include_archived=True + ) + if children is None: + logger.error( + "Failed to fetch children for incident %s", incident.incident_number + ) + failed += 1 + continue + for issue in children: + if issue["id"] in seen: + continue + seen.add(issue["id"]) + if label_id in issue.get("label_ids", []): + continue + missing += 1 + logger.info( + "%s %s to %s", + "Adding" if args.apply else "Would add", + INCIDENT_ACTION_LABEL_NAME, + issue["identifier"], + ) + if args.apply: + if service.add_issue_label(issue["id"], label_id): + added += 1 + else: + logger.error("Failed to label %s", issue["identifier"]) + failed += 1 + + logger.info( + "%s: %d checked, %d missing labels, %d added, %d failures", + "Backfill" if args.apply else "Dry run", + len(seen), + missing, + added, + failed, + ) + return 1 if failed else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/firetower/incidents/services.py b/src/firetower/incidents/services.py index 39d70520..e20f7797 100644 --- a/src/firetower/incidents/services.py +++ b/src/firetower/incidents/services.py @@ -21,6 +21,7 @@ IncidentStatus, ) from firetower.integrations.services import LinearService, SlackService +from firetower.integrations.services.linear import INCIDENT_ACTION_LABEL_NAME logger = logging.getLogger(__name__) _slack_service = SlackService() @@ -186,6 +187,38 @@ def _set_action_item_sla(linear_service: LinearService, issue: dict) -> None: logger.error("Failed to set Linear SLA for action item %s", issue["identifier"]) +def _set_action_item_labels( + linear_service: LinearService, issues: list[dict] +) -> list[str]: + if not issues: + return [] + + try: + label_id = linear_service.get_issue_label_id(INCIDENT_ACTION_LABEL_NAME) + except Exception: + logger.exception("Failed to fetch Linear incident action label") + return ["Failed to fetch Linear incident action label"] + if not label_id: + error = f"Linear workspace label {INCIDENT_ACTION_LABEL_NAME} is unavailable" + logger.warning(error) + return [error] + + errors = [] + for issue in issues: + if label_id in issue.get("label_ids", []): + continue + try: + success = linear_service.add_issue_label(issue["id"], label_id) + except Exception: + logger.exception("Failed to label action item %s", issue["identifier"]) + success = False + if not success: + error = f"Failed to label action item {issue['identifier']}" + logger.warning(error) + errors.append(error) + return errors + + def _resolve_assignees( issues: dict[str, dict], ) -> dict[str, User | None]: @@ -390,6 +423,10 @@ def sync_action_items_from_linear( incident.action_items_last_synced_at = timezone.now() incident.save(update_fields=["action_items_last_synced_at"]) + stats.errors.extend( + _set_action_item_labels(linear_service, list(all_issues.values())) + ) + if incident.severity in (IncidentSeverity.P0, IncidentSeverity.P1): for issue in all_issues.values(): _set_action_item_sla(linear_service, issue) diff --git a/src/firetower/incidents/tests/test_action_item_labels.py b/src/firetower/incidents/tests/test_action_item_labels.py new file mode 100644 index 00000000..8776515c --- /dev/null +++ b/src/firetower/incidents/tests/test_action_item_labels.py @@ -0,0 +1,249 @@ +import runpy +from datetime import timedelta +from pathlib import Path +from unittest.mock import MagicMock, call, patch + +import pytest +from django.utils import timezone + +from firetower.incidents.models import ( + ActionItem, + Incident, + IncidentSeverity, + IncidentStatus, +) +from firetower.incidents.services import sync_action_items_from_linear +from firetower.integrations.services.linear import LinearService + +pytestmark = pytest.mark.django_db + + +@pytest.fixture +def issue(): + return { + "id": "issue-1", + "identifier": "ENG-1", + "title": "Pre-existing follow-up", + "url": "https://linear.app/issue/ENG-1", + "status": "Todo", + "priority": 0, + "relation_type": "child", + "created_at": timezone.now() - timedelta(days=365), + "label_ids": ["unrelated-label"], + } + + +@pytest.fixture +def service(settings, issue): + settings.LINEAR = {"TEAM_ID": "incident-team"} + service = MagicMock(spec=LinearService) + service.get_child_issues.return_value = [issue] + service.get_issue_label_id.return_value = "action-label" + service.add_issue_label.return_value = True + with ( + patch("firetower.incidents.services._get_linear_service", return_value=service), + patch( + "firetower.integrations.services.linear.LinearService", return_value=service + ), + ): + yield service + + +@pytest.fixture +def incident(): + return Incident.objects.create( + title="Incident", + severity=IncidentSeverity.P2, + status=IncidentStatus.POSTMORTEM, + linear_parent_issue_id="parent-1", + ) + + +@pytest.fixture +def backfill_main(): + script = ( + Path(__file__).resolve().parents[4] + / "scripts/backfill_incident_action_labels.py" + ) + with patch("django.setup"): + yield runpy.run_path(str(script))["main"] + + +class TestActionItemLabels: + @pytest.mark.parametrize("already_synced", [False, True]) + def test_labels_newly_adopted_and_cached_items( + self, incident, issue, service, already_synced + ): + if already_synced: + ActionItem.objects.create( + incident=incident, + linear_issue_id=issue["id"], + linear_identifier=issue["identifier"], + title="Old title", + url=issue["url"], + ) + + stats = sync_action_items_from_linear(incident, force=True) + + assert stats.errors == [] + item = incident.action_items.get() + assert item.created_at == issue["created_at"] + service.add_issue_label.assert_called_once_with("issue-1", "action-label") + service.get_issue_label_id.assert_called_once_with("incident-action") + + @pytest.mark.parametrize("severity", IncidentSeverity.values) + @pytest.mark.parametrize("status", ["Todo", "In Progress", "Done", "Canceled"]) + def test_labels_every_severity_and_action_status( + self, incident, issue, service, severity, status + ): + incident.severity = severity + issue["status"] = status + sync_action_items_from_linear(incident, force=True) + service.add_issue_label.assert_called_once_with("issue-1", "action-label") + + def test_skips_already_labeled_items(self, incident, issue, service): + issue["label_ids"].append("action-label") + stats = sync_action_items_from_linear(incident, force=True) + assert stats.created == 1 + assert stats.errors == [] + service.add_issue_label.assert_not_called() + + @pytest.mark.parametrize("failure", [False, RuntimeError("Label update failed")]) + def test_label_failure_does_not_block_sync_or_other_items( + self, incident, issue, service, failure + ): + service.get_child_issues.return_value = [ + issue, + {**issue, "id": "issue-2", "identifier": "ENG-2"}, + ] + service.add_issue_label.side_effect = [failure, True] + + stats = sync_action_items_from_linear(incident, force=True) + + assert stats.created == 2 + assert stats.errors == ["Failed to label action item ENG-1"] + assert incident.action_items.count() == 2 + service.add_issue_label.assert_has_calls( + [call("issue-1", "action-label"), call("issue-2", "action-label")] + ) + + service.add_issue_label.side_effect = None + stats = sync_action_items_from_linear(incident, force=True) + assert stats.errors == [] + assert stats.updated == 2 + + @pytest.mark.parametrize("failure", [None, RuntimeError("Label lookup failed")]) + def test_missing_label_does_not_block_sync(self, incident, service, failure): + if isinstance(failure, Exception): + service.get_issue_label_id.side_effect = failure + else: + service.get_issue_label_id.return_value = failure + + stats = sync_action_items_from_linear(incident, force=True) + + assert stats.created == 1 + assert len(stats.errors) == 1 + service.add_issue_label.assert_not_called() + + def test_does_not_fetch_label_without_children(self, incident, service): + service.get_child_issues.return_value = [] + sync_action_items_from_linear(incident, force=True) + service.get_issue_label_id.assert_not_called() + service.add_issue_label.assert_not_called() + + +class TestBackfillActionItemLabels: + def test_dry_run_does_not_mutate_issues_or_cached_items( + self, incident, service, backfill_main + ): + assert backfill_main([]) == 0 + service.get_child_issues.assert_called_once_with( + "parent-1", include_archived=True + ) + service.add_issue_label.assert_not_called() + service.update_issue.assert_not_called() + service.create_comment.assert_not_called() + assert incident.action_items.count() == 0 + + def test_apply_skips_existing_labels(self, incident, issue, service, backfill_main): + service.get_child_issues.return_value = [ + issue, + { + **issue, + "id": "issue-2", + "identifier": "ENG-2", + "label_ids": ["action-label"], + }, + ] + assert backfill_main(["--apply"]) == 0 + service.add_issue_label.assert_called_once_with("issue-1", "action-label") + service.update_issue.assert_not_called() + assert incident.action_items.count() == 0 + + issue["label_ids"].append("action-label") + assert backfill_main(["--apply"]) == 0 + service.add_issue_label.assert_called_once_with("issue-1", "action-label") + + def test_includes_historical_private_incidents_and_deduplicates( + self, incident, issue, service, backfill_main + ): + old_incident = Incident.objects.create( + title="Old private incident", + severity=IncidentSeverity.P4, + status=IncidentStatus.CANCELED, + is_private=True, + linear_parent_issue_id="parent-2", + ) + Incident.objects.filter(pk=old_incident.pk).update( + created_at=timezone.now() - timedelta(days=365) + ) + Incident.objects.create(title="No Linear parent", severity=IncidentSeverity.P2) + Incident.objects.create( + title="Empty Linear parent", + severity=IncidentSeverity.P2, + linear_parent_issue_id="", + ) + + assert backfill_main(["--apply"]) == 0 + + service.get_child_issues.assert_has_calls( + [ + call("parent-1", include_archived=True), + call("parent-2", include_archived=True), + ], + any_order=True, + ) + assert service.get_child_issues.call_count == 2 + service.add_issue_label.assert_called_once_with("issue-1", "action-label") + + def test_reports_failed_additions(self, incident, service, backfill_main): + service.add_issue_label.return_value = False + assert backfill_main(["--apply"]) == 1 + + def test_reports_incomplete_fetch_and_continues( + self, incident, service, backfill_main + ): + Incident.objects.create( + title="Another incident", + severity=IncidentSeverity.P2, + linear_parent_issue_id="parent-2", + ) + service.get_child_issues.side_effect = [ + None, + service.get_child_issues.return_value, + ] + assert backfill_main(["--apply"]) == 1 + assert service.get_child_issues.call_count == 2 + service.add_issue_label.assert_called_once_with("issue-1", "action-label") + + def test_stops_when_label_is_missing(self, service, backfill_main): + service.get_issue_label_id.return_value = None + assert backfill_main(["--apply"]) == 1 + service.get_child_issues.assert_not_called() + service.add_issue_label.assert_not_called() + + def test_requires_linear_configuration(self, settings, backfill_main): + settings.LINEAR = None + with pytest.raises(SystemExit) as error: + backfill_main([]) + assert error.value.code == 2 diff --git a/src/firetower/integrations/services/linear.py b/src/firetower/integrations/services/linear.py index 74ccf0f5..0223c39d 100644 --- a/src/firetower/integrations/services/linear.py +++ b/src/firetower/integrations/services/linear.py @@ -16,6 +16,7 @@ LINEAR_API_URL = "https://api.linear.app/graphql" LINEAR_TOKEN_URL = "https://api.linear.app/oauth/token" +INCIDENT_ACTION_LABEL_NAME = "incident-action" LINEAR_STATE_TYPE_MAP = { "triage": "Todo", @@ -46,6 +47,11 @@ priority createdAt slaBreachesAt + labels { + nodes { + id + } + } state { id type @@ -163,6 +169,7 @@ def __init__( timeout if timeout is not None else LINEAR_DEFAULT_TIMEOUT_SECONDS ) self._workflow_states_cache: dict[str, str] | None = None + self._issue_label_ids_cache: dict[str, str] = {} @classmethod def for_allocation(cls) -> "LinearService": @@ -406,6 +413,9 @@ def _parse_issue( "url": issue["url"], "status": status, "priority": issue.get("priority", 0), + "label_ids": [ + label["id"] for label in (issue.get("labels") or {}).get("nodes", []) + ], "created_at": parse_datetime(created_at) if created_at else None, "sla_breaches_at": ( parse_datetime(sla_breaches_at) if sla_breaches_at else None @@ -527,6 +537,53 @@ def create_issue( "url": issue["url"], } + def get_issue_label_id(self, name: str) -> str | None: + """Find an existing workspace-wide label that can be used across teams.""" + if name in self._issue_label_ids_cache: + return self._issue_label_ids_cache[name] + + query = """ + query($name: String!) { + issueLabels( + filter: { + name: { eq: $name }, + team: { null: true }, + isGroup: { eq: false } + }, + first: 2 + ) { + nodes { + id + } + } + } + """ + data = self._graphql(query, {"name": name}) + if not data: + return None + labels = data.get("issueLabels", {}).get("nodes", []) + if len(labels) != 1: + logger.warning("Expected one Linear workspace label named %s", name) + return None + + label_id = labels[0]["id"] + self._issue_label_ids_cache[name] = label_id + return label_id + + def add_issue_label(self, issue_id: str, label_id: str) -> bool: + """Add a label atomically without replacing any existing issue labels.""" + mutation = """ + mutation($id: String!, $labelId: String!) { + issueAddLabel(id: $id, labelId: $labelId) { + success + } + } + """ + data = self._graphql(mutation, {"id": issue_id, "labelId": label_id}) + if not data: + return False + return data.get("issueAddLabel", {}).get("success", False) + def create_attachment(self, issue_id: str, url: str, title: str) -> bool: mutation = """ mutation($input: AttachmentCreateInput!) { @@ -706,11 +763,13 @@ def get_workflow_states(self, team_id: str) -> dict[str, str] | None: self._workflow_states_cache = states return states - def get_child_issues(self, issue_id: str) -> list[dict[str, Any]] | None: + def get_child_issues( + self, issue_id: str, *, include_archived: bool = False + ) -> list[dict[str, Any]] | None: query = f""" - query($issueId: String!, $after: String) {{ + query($issueId: String!, $after: String, $includeArchived: Boolean!) {{ issue(id: $issueId) {{ - children(first: 50, after: $after) {{ + children(first: 50, after: $after, includeArchived: $includeArchived) {{ nodes {{ {ISSUE_FIELDS} }} @@ -728,7 +787,10 @@ def get_child_issues(self, issue_id: str) -> list[dict[str, Any]] | None: max_pages = 25 for _ in range(max_pages): - variables: dict[str, Any] = {"issueId": issue_id} + variables: dict[str, Any] = { + "issueId": issue_id, + "includeArchived": include_archived, + } if cursor is not None: variables["after"] = cursor @@ -747,9 +809,14 @@ def get_child_issues(self, issue_id: str) -> list[dict[str, Any]] | None: page_info = children.get("pageInfo", {}) if not page_info.get("hasNextPage"): - break - cursor = page_info.get("endCursor") - if cursor is None: - break + return issues + next_cursor = page_info.get("endCursor") + if not next_cursor or next_cursor == cursor: + logger.warning( + "Incomplete Linear child issue pagination for %s", issue_id + ) + return None + cursor = next_cursor - return issues + logger.warning("Reached Linear child issue page limit for %s", issue_id) + return None diff --git a/src/firetower/integrations/tests/test_linear_service.py b/src/firetower/integrations/tests/test_linear_service.py index 88e5fadf..b504f48f 100644 --- a/src/firetower/integrations/tests/test_linear_service.py +++ b/src/firetower/integrations/tests/test_linear_service.py @@ -779,6 +779,7 @@ def test_parses_full_issue(self, linear_service): "url": "https://linear.app/team/issue/ENG-123", "status": "In Progress", "priority": 2, + "label_ids": [], "created_at": datetime(2026, 10, 16, 12, 30, tzinfo=UTC), "sla_breaches_at": datetime(2026, 10, 30, 12, 30, tzinfo=UTC), "assignee_email": "alice@sentry.io", @@ -786,6 +787,19 @@ def test_parses_full_issue(self, linear_service): "relation_type": "child", } + def test_parses_existing_label_ids(self, linear_service): + issue = { + "id": "issue-1", + "identifier": "ENG-1", + "title": "An existing issue", + "url": "https://linear.app/issue/ENG-1", + "labels": {"nodes": [{"id": "existing-label"}, {"id": "action-label"}]}, + } + assert linear_service._parse_issue(issue)["label_ids"] == [ + "existing-label", + "action-label", + ] + def test_maps_all_state_types(self, linear_service): for state_type, expected_status in [ ("triage", "Todo"), @@ -806,7 +820,102 @@ def test_maps_all_state_types(self, linear_service): assert linear_service._parse_issue(issue)["status"] == expected_status +class TestIssueLabels: + def test_finds_and_caches_workspace_label(self, linear_service): + response = {"issueLabels": {"nodes": [{"id": "action-label"}]}} + with patch.object(linear_service, "_graphql", return_value=response) as graphql: + assert ( + linear_service.get_issue_label_id("incident-action") == "action-label" + ) + assert ( + linear_service.get_issue_label_id("incident-action") == "action-label" + ) + + graphql.assert_called_once() + query, variables = graphql.call_args.args + assert variables == {"name": "incident-action"} + assert "team: { null: true }" in query + assert "isGroup: { eq: false }" in query + + @pytest.mark.parametrize( + "response", + [ + None, + {"issueLabels": {"nodes": []}}, + {"issueLabels": {"nodes": [{"id": "a"}, {"id": "b"}]}}, + ], + ) + def test_does_not_cache_missing_or_ambiguous_label(self, linear_service, response): + with patch.object(linear_service, "_graphql", return_value=response) as graphql: + assert linear_service.get_issue_label_id("incident-action") is None + assert linear_service.get_issue_label_id("incident-action") is None + assert graphql.call_count == 2 + + def test_adds_label_without_replacing_existing_labels(self, linear_service): + with patch.object( + linear_service, + "_graphql", + return_value={"issueAddLabel": {"success": True}}, + ) as graphql: + assert linear_service.add_issue_label("issue-1", "action-label") is True + + query, variables = graphql.call_args.args + assert "issueAddLabel" in query + assert variables == {"id": "issue-1", "labelId": "action-label"} + + @pytest.mark.parametrize("response", [None, {"issueAddLabel": {"success": False}}]) + def test_reports_addition_failure(self, linear_service, response): + with patch.object(linear_service, "_graphql", return_value=response): + assert linear_service.add_issue_label("issue-1", "action-label") is False + + class TestGetChildIssues: + def test_includes_archived_children_when_requested(self, linear_service): + response = { + "issue": {"children": {"nodes": [], "pageInfo": {"hasNextPage": False}}} + } + with patch.object(linear_service, "_graphql", return_value=response) as graphql: + assert ( + linear_service.get_child_issues("parent-id", include_archived=True) + == [] + ) + assert graphql.call_args.args[1]["includeArchived"] is True + assert "includeArchived: $includeArchived" in graphql.call_args.args[0] + + @pytest.mark.parametrize("end_cursor", [None, "cursor1"]) + def test_rejects_incomplete_pagination(self, linear_service, end_cursor): + def page(cursor): + return { + "issue": { + "children": { + "nodes": [], + "pageInfo": {"hasNextPage": True, "endCursor": cursor}, + } + } + } + + with patch.object( + linear_service, "_graphql", side_effect=[page("cursor1"), page(end_cursor)] + ): + assert linear_service.get_child_issues("parent-id") is None + + def test_rejects_page_limit_instead_of_returning_partial_children( + self, linear_service + ): + pages = [ + { + "issue": { + "children": { + "nodes": [], + "pageInfo": {"hasNextPage": True, "endCursor": str(index)}, + } + } + } + for index in range(25) + ] + with patch.object(linear_service, "_graphql", side_effect=pages): + assert linear_service.get_child_issues("parent-id") is None + def test_paginates_through_multiple_pages(self, linear_service): with patch.object( linear_service, From 0b8fc34c64187d5d94c52b144018f916c4a297a4 Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Tue, 27 Oct 2026 14:11:14 -0400 Subject: [PATCH 2/7] Expose incident action label backfills in Django admin --- README.md | 14 ++- scripts/backfill_incident_action_labels.py | 61 +----------- src/firetower/incidents/admin.py | 37 ++++++- src/firetower/incidents/services.py | 91 +++++++++++++++++ .../tests/test_action_item_labels.py | 80 +++++++++++++-- src/firetower/incidents/tests/test_admin.py | 99 ++++++++++++++++++- 6 files changed, 310 insertions(+), 72 deletions(-) diff --git a/README.md b/README.md index 42dbb568..fd189d91 100644 --- a/README.md +++ b/README.md @@ -131,17 +131,15 @@ sdk/ # Python SDK Action item sync adds the existing workspace-wide `incident-action` label to incident sub-issues, including pre-existing issues adopted as follow-ups. Other labels are preserved. -To backfill historical action items, use the script with the intended environment's Firetower configuration. It reads current sub-issues for every stored incident parent, including archived children, without changing parent relationships, SLAs, or cached action items. +To backfill historical action items in production, open **Django admin → Incidents**: -```sh -# Preview label additions without updating issues -uv run python scripts/backfill_incident_action_labels.py +1. Select a batch of incidents. +2. Run **Preview incident-action label backfill** to see how many labels are missing without updating Linear issues. +3. Select the same incidents and run **Backfill incident-action labels in Linear** to apply the additions. -# Apply label additions after reviewing the preview -uv run python scripts/backfill_incident_action_labels.py --apply -``` +Both actions require permission to change incidents and report counts and failures in admin. They read current sub-issues for the selected incident parents, including archived children, without changing parent relationships, SLAs, or cached action items. Use manageable batches to keep admin requests short; rerunning safely skips existing labels. The workspace label must already exist. -The backfill is safe to rerun and exits nonzero on failed lookups or label updates. It does not create the workspace label if it is missing. +The optional local script, `scripts/backfill_incident_action_labels.py`, uses the same backfill logic and still defaults to a dry run. ## Scheduled Tasks diff --git a/scripts/backfill_incident_action_labels.py b/scripts/backfill_incident_action_labels.py index d4c8a99e..0aa4462f 100644 --- a/scripts/backfill_incident_action_labels.py +++ b/scripts/backfill_incident_action_labels.py @@ -11,8 +11,6 @@ import django from django.conf import settings -logger = logging.getLogger(__name__) - def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser(description=__doc__) @@ -23,64 +21,15 @@ def main(argv: list[str] | None = None) -> int: django.setup() from firetower.incidents.models import Incident # noqa: PLC0415 - from firetower.integrations.services.linear import ( # noqa: PLC0415 - INCIDENT_ACTION_LABEL_NAME, - LinearService, - ) + from firetower.incidents.services import backfill_action_item_labels # noqa: PLC0415 if not settings.LINEAR: parser.error("Linear integration is not configured") - service = LinearService() - label_id = service.get_issue_label_id(INCIDENT_ACTION_LABEL_NAME) - if not label_id: - logger.error("Workspace label %s is unavailable", INCIDENT_ACTION_LABEL_NAME) - return 1 - - incidents = Incident.objects.exclude(linear_parent_issue_id__isnull=True).exclude( - linear_parent_issue_id="" - ) - seen: set[str] = set() - missing = added = failed = 0 - for incident in incidents.iterator(): - children = service.get_child_issues( - incident.linear_parent_issue_id, include_archived=True - ) - if children is None: - logger.error( - "Failed to fetch children for incident %s", incident.incident_number - ) - failed += 1 - continue - for issue in children: - if issue["id"] in seen: - continue - seen.add(issue["id"]) - if label_id in issue.get("label_ids", []): - continue - missing += 1 - logger.info( - "%s %s to %s", - "Adding" if args.apply else "Would add", - INCIDENT_ACTION_LABEL_NAME, - issue["identifier"], - ) - if args.apply: - if service.add_issue_label(issue["id"], label_id): - added += 1 - else: - logger.error("Failed to label %s", issue["identifier"]) - failed += 1 - - logger.info( - "%s: %d checked, %d missing labels, %d added, %d failures", - "Backfill" if args.apply else "Dry run", - len(seen), - missing, - added, - failed, - ) - return 1 if failed else 0 + stats = backfill_action_item_labels(Incident.objects.all(), dry_run=not args.apply) + for error in stats.errors: + logging.getLogger(__name__).error("%s", error) + return 1 if stats.errors else 0 if __name__ == "__main__": diff --git a/src/firetower/incidents/admin.py b/src/firetower/incidents/admin.py index 0db98a2c..65249323 100644 --- a/src/firetower/incidents/admin.py +++ b/src/firetower/incidents/admin.py @@ -1,11 +1,12 @@ from __future__ import annotations -from django.contrib import admin +from django.contrib import admin, messages from django.db.models import QuerySet from django.http import HttpRequest from .models import ExternalLink, Incident, Tag from .services import ( + backfill_action_item_labels, sync_action_items_from_linear, sync_incident_participants_from_slack, ) @@ -42,6 +43,8 @@ class IncidentAdmin(admin.ModelAdmin): actions = [ "sync_participants_from_slack", "sync_action_items", + "preview_action_item_labels", + "backfill_action_item_labels", "clear_milestones", ] @@ -149,6 +152,38 @@ def sync_action_items( self.message_user(request, f"Action item sync: {', '.join(message_parts)}") + @admin.action( + description="Preview incident-action label backfill", permissions=["change"] + ) + def preview_action_item_labels( + self, request: HttpRequest, queryset: QuerySet[Incident] + ) -> None: + self._backfill_action_item_labels(request, queryset, dry_run=True) + + @admin.action( + description="Backfill incident-action labels in Linear", permissions=["change"] + ) + def backfill_action_item_labels( + self, request: HttpRequest, queryset: QuerySet[Incident] + ) -> None: + self._backfill_action_item_labels(request, queryset, dry_run=False) + + def _backfill_action_item_labels( + self, request: HttpRequest, queryset: QuerySet[Incident], *, dry_run: bool + ) -> None: + stats = backfill_action_item_labels(queryset, dry_run=dry_run) + operation = "preview" if dry_run else "backfill" + message = ( + f"Action label {operation}: {stats.checked} checked, " + f"{stats.missing} missing labels, {stats.added} added, " + f"{len(stats.errors)} failures." + ) + if stats.errors: + message += f" {stats.errors[0]}" + self.message_user( + request, message, level=messages.ERROR if stats.errors else messages.SUCCESS + ) + @admin.action(description="Clear all milestones") def clear_milestones( self, request: HttpRequest, queryset: QuerySet[Incident] diff --git a/src/firetower/incidents/services.py b/src/firetower/incidents/services.py index e20f7797..6ebccfbd 100644 --- a/src/firetower/incidents/services.py +++ b/src/firetower/incidents/services.py @@ -5,6 +5,7 @@ from django.conf import settings from django.contrib.auth.models import User from django.db import transaction +from django.db.models import QuerySet from django.utils import timezone from firetower.auth.models import ExternalProfile, ExternalProfileType @@ -150,6 +151,96 @@ class ActionItemsSyncStats: skipped: bool = False +@dataclass +class ActionItemLabelBackfillStats: + checked: int = 0 + missing: int = 0 + added: int = 0 + errors: list[str] = field(default_factory=list) + + +def backfill_action_item_labels( + incidents: QuerySet[Incident], *, dry_run: bool = True +) -> ActionItemLabelBackfillStats: + """Label live incident sub-issues without running other sync side effects.""" + stats = ActionItemLabelBackfillStats() + if not settings.LINEAR: + stats.errors.append("Linear integration is not configured") + return stats + + try: + service = _get_linear_service() + label_id = service.get_issue_label_id(INCIDENT_ACTION_LABEL_NAME) + except Exception: + logger.exception("Failed to fetch Linear incident action label") + stats.errors.append("Failed to fetch Linear incident action label") + return stats + if not label_id: + stats.errors.append( + f"Linear workspace label {INCIDENT_ACTION_LABEL_NAME} is unavailable" + ) + return stats + + incidents = incidents.exclude(linear_parent_issue_id__isnull=True).exclude( + linear_parent_issue_id="" + ) + seen: set[str] = set() + for incident in incidents.iterator(): + assert incident.linear_parent_issue_id is not None + try: + children = service.get_child_issues( + incident.linear_parent_issue_id, include_archived=True + ) + except Exception: + logger.exception( + "Failed to fetch children for incident %s", incident.incident_number + ) + children = None + if children is None: + stats.errors.append( + f"Failed to fetch children for incident {incident.incident_number}" + ) + continue + + for issue in children: + if issue["id"] in seen: + continue + seen.add(issue["id"]) + stats.checked += 1 + if label_id in issue.get("label_ids", []): + continue + stats.missing += 1 + logger.info( + "%s %s to %s", + "Would add" if dry_run else "Adding", + INCIDENT_ACTION_LABEL_NAME, + issue["identifier"], + ) + if dry_run: + continue + try: + success = service.add_issue_label(issue["id"], label_id) + except Exception: + logger.exception("Failed to label action item %s", issue["identifier"]) + success = False + if success: + stats.added += 1 + else: + stats.errors.append( + f"Failed to label action item {issue['identifier']}" + ) + + logger.info( + "%s: %d checked, %d missing labels, %d added, %d failures", + "Dry run" if dry_run else "Backfill", + stats.checked, + stats.missing, + stats.added, + len(stats.errors), + ) + return stats + + def _get_action_item_slo_days(priority: int) -> int | None: linear = settings.LINEAR or {} if priority in (1, 2): diff --git a/src/firetower/incidents/tests/test_action_item_labels.py b/src/firetower/incidents/tests/test_action_item_labels.py index 8776515c..af4adbca 100644 --- a/src/firetower/incidents/tests/test_action_item_labels.py +++ b/src/firetower/incidents/tests/test_action_item_labels.py @@ -12,7 +12,10 @@ IncidentSeverity, IncidentStatus, ) -from firetower.incidents.services import sync_action_items_from_linear +from firetower.incidents.services import ( + backfill_action_item_labels, + sync_action_items_from_linear, +) from firetower.integrations.services.linear import LinearService pytestmark = pytest.mark.django_db @@ -40,11 +43,8 @@ def service(settings, issue): service.get_child_issues.return_value = [issue] service.get_issue_label_id.return_value = "action-label" service.add_issue_label.return_value = True - with ( - patch("firetower.incidents.services._get_linear_service", return_value=service), - patch( - "firetower.integrations.services.linear.LinearService", return_value=service - ), + with patch( + "firetower.incidents.services._get_linear_service", return_value=service ): yield service @@ -153,6 +153,74 @@ def test_does_not_fetch_label_without_children(self, incident, service): class TestBackfillActionItemLabels: + def test_respects_selected_incidents(self, incident, service): + Incident.objects.create( + title="Not selected", + severity=IncidentSeverity.P2, + linear_parent_issue_id="parent-2", + ) + stats = backfill_action_item_labels( + Incident.objects.filter(pk=incident.pk), dry_run=False + ) + assert stats.checked == stats.missing == stats.added == 1 + assert stats.errors == [] + service.get_child_issues.assert_called_once_with( + "parent-1", include_archived=True + ) + + def test_preview_reports_counts_without_adding_labels( + self, incident, issue, service + ): + service.get_child_issues.return_value = [ + issue, + {**issue, "id": "issue-2", "label_ids": ["action-label"]}, + ] + stats = backfill_action_item_labels(Incident.objects.all()) + assert stats.checked == 2 + assert stats.missing == 1 + assert stats.added == 0 + assert stats.errors == [] + service.add_issue_label.assert_not_called() + + def test_continues_after_unexpected_child_fetch_error(self, incident, service): + Incident.objects.create( + title="Another incident", + severity=IncidentSeverity.P2, + linear_parent_issue_id="parent-2", + ) + service.get_child_issues.side_effect = [ + RuntimeError("Fetch failed"), + service.get_child_issues.return_value, + ] + stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) + assert stats.checked == stats.added == 1 + assert len(stats.errors) == 1 + assert service.get_child_issues.call_count == 2 + + def test_continues_after_unexpected_label_error(self, incident, issue, service): + service.get_child_issues.return_value = [ + issue, + {**issue, "id": "issue-2", "identifier": "ENG-2"}, + ] + service.add_issue_label.side_effect = [RuntimeError("Update failed"), True] + stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) + assert stats.checked == stats.missing == 2 + assert stats.added == 1 + assert stats.errors == ["Failed to label action item ENG-1"] + + def test_reports_label_lookup_exception(self, service): + service.get_issue_label_id.side_effect = RuntimeError("Lookup failed") + stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) + assert stats.errors == ["Failed to fetch Linear incident action label"] + service.get_child_issues.assert_not_called() + service.add_issue_label.assert_not_called() + + def test_reports_missing_configuration(self, settings, service): + settings.LINEAR = None + stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) + assert stats.errors == ["Linear integration is not configured"] + service.get_issue_label_id.assert_not_called() + def test_dry_run_does_not_mutate_issues_or_cached_items( self, incident, service, backfill_main ): diff --git a/src/firetower/incidents/tests/test_admin.py b/src/firetower/incidents/tests/test_admin.py index dca32b76..e6bcd2ab 100644 --- a/src/firetower/incidents/tests/test_admin.py +++ b/src/firetower/incidents/tests/test_admin.py @@ -1,6 +1,7 @@ from unittest.mock import patch import pytest +from django.contrib import messages from django.contrib.admin.sites import AdminSite from django.contrib.auth.models import User from django.contrib.messages.storage.fallback import FallbackStorage @@ -8,7 +9,10 @@ from firetower.incidents.admin import IncidentAdmin from firetower.incidents.models import Incident, IncidentSeverity, IncidentStatus -from firetower.incidents.services import ParticipantsSyncStats +from firetower.incidents.services import ( + ActionItemLabelBackfillStats, + ParticipantsSyncStats, +) @pytest.mark.django_db @@ -35,6 +39,99 @@ def test_sync_participants_action_exists(self): actions = self.incident_admin.get_actions(request) assert "sync_participants_from_slack" in actions + def test_action_label_actions_exist(self): + request = self._add_session_and_messages(self.factory.get("/")) + actions = self.incident_admin.get_actions(request) + assert "preview_action_item_labels" in actions + assert "backfill_action_item_labels" in actions + + def test_action_label_actions_require_change_permission(self): + request = self._add_session_and_messages(self.factory.get("/")) + request.user = User.objects.create_user(username="read-only", is_staff=True) + actions = self.incident_admin.get_actions(request) + assert "preview_action_item_labels" not in actions + assert "backfill_action_item_labels" not in actions + + @pytest.mark.parametrize( + ("action", "dry_run"), + [("preview_action_item_labels", True), ("backfill_action_item_labels", False)], + ) + def test_action_label_admin_post_limits_backfill_to_selection( + self, client, action, dry_run + ): + selected = Incident.objects.create( + title="Selected", severity=IncidentSeverity.P2 + ) + Incident.objects.create(title="Not selected", severity=IncidentSeverity.P2) + client.force_login(self.admin_user) + + with patch("firetower.incidents.admin.backfill_action_item_labels") as backfill: + backfill.return_value = ActionItemLabelBackfillStats() + response = client.post( + "/admin/incidents/incident/", + {"action": action, "_selected_action": [str(selected.pk)]}, + ) + + assert response.status_code == 302 + backfill.assert_called_once() + assert list(backfill.call_args.args[0]) == [selected] + assert backfill.call_args.kwargs == {"dry_run": dry_run} + + @pytest.mark.parametrize( + ("action", "dry_run"), + [("preview_action_item_labels", True), ("backfill_action_item_labels", False)], + ) + def test_action_label_actions_report_counts(self, action, dry_run): + request = self._add_session_and_messages(self.factory.post("/")) + queryset = Incident.objects.none() + with ( + patch("firetower.incidents.admin.backfill_action_item_labels") as backfill, + patch.object(self.incident_admin, "message_user") as message_user, + ): + backfill.return_value = ActionItemLabelBackfillStats( + checked=3, missing=2, added=0 if dry_run else 2 + ) + getattr(self.incident_admin, action)(request, queryset) + + backfill.assert_called_once_with(queryset, dry_run=dry_run) + message = message_user.call_args.args[1] + assert "3 checked" in message + assert "2 missing labels" in message + assert f"{0 if dry_run else 2} added" in message + assert "0 failures" in message + assert message_user.call_args.kwargs["level"] == messages.SUCCESS + + def test_action_label_backfill_reports_partial_failure(self): + request = self._add_session_and_messages(self.factory.post("/")) + with ( + patch("firetower.incidents.admin.backfill_action_item_labels") as backfill, + patch.object(self.incident_admin, "message_user") as message_user, + ): + backfill.return_value = ActionItemLabelBackfillStats( + checked=3, + missing=2, + added=1, + errors=["Failed to label action item ENG-1"], + ) + self.incident_admin.backfill_action_item_labels( + request, Incident.objects.none() + ) + + message = message_user.call_args.args[1] + assert "1 added" in message + assert "1 failures" in message + assert "Failed to label action item ENG-1" in message + assert message_user.call_args.kwargs["level"] == messages.ERROR + + def test_action_label_backfill_reports_missing_configuration(self): + request = self._add_session_and_messages(self.factory.post("/")) + with patch.object(self.incident_admin, "message_user") as message_user: + self.incident_admin.backfill_action_item_labels( + request, Incident.objects.none() + ) + assert "Linear integration is not configured" in message_user.call_args.args[1] + assert message_user.call_args.kwargs["level"] == messages.ERROR + def test_sync_participants_action_calls_sync_function(self): incident1 = Incident.objects.create( title="Test Incident 1", From c44d3cf6f8933773d503386c47983f4ad977e286 Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Tue, 27 Oct 2026 14:44:19 -0400 Subject: [PATCH 3/7] Stop action label backfills when Linear rate limits requests --- README.md | 2 + src/firetower/incidents/services.py | 30 ++++- .../tests/test_action_item_labels.py | 111 ++++++++++++++++-- src/firetower/integrations/services/linear.py | 70 ++++++++++- .../integrations/tests/test_linear_service.py | 85 ++++++++++++++ 5 files changed, 279 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index fd189d91..fec756e7 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,8 @@ To backfill historical action items in production, open **Django admin → Incid Both actions require permission to change incidents and report counts and failures in admin. They read current sub-issues for the selected incident parents, including archived children, without changing parent relationships, SLAs, or cached action items. Use manageable batches to keep admin requests short; rerunning safely skips existing labels. The workspace label must already exist. +If Linear rate-limits a request, the action stops immediately and reports partial progress and any `Retry-After` guidance. It does not wait for quota to reset inside the admin request or resume automatically. Wait for the limit to reset, then rerun the same selection; issues already labeled are skipped. + The optional local script, `scripts/backfill_incident_action_labels.py`, uses the same backfill logic and still defaults to a dry run. ## Scheduled Tasks diff --git a/src/firetower/incidents/services.py b/src/firetower/incidents/services.py index 6ebccfbd..d7a5b564 100644 --- a/src/firetower/incidents/services.py +++ b/src/firetower/incidents/services.py @@ -22,7 +22,10 @@ IncidentStatus, ) from firetower.integrations.services import LinearService, SlackService -from firetower.integrations.services.linear import INCIDENT_ACTION_LABEL_NAME +from firetower.integrations.services.linear import ( + INCIDENT_ACTION_LABEL_NAME, + LinearRateLimitError, +) logger = logging.getLogger(__name__) _slack_service = SlackService() @@ -170,7 +173,12 @@ def backfill_action_item_labels( try: service = _get_linear_service() - label_id = service.get_issue_label_id(INCIDENT_ACTION_LABEL_NAME) + label_id = service.get_issue_label_id( + INCIDENT_ACTION_LABEL_NAME, raise_on_error=True + ) + except LinearRateLimitError as error: + stats.errors.insert(0, f"{error} Backfill stopped; rerun this selection later.") + return stats except Exception: logger.exception("Failed to fetch Linear incident action label") stats.errors.append("Failed to fetch Linear incident action label") @@ -189,8 +197,15 @@ def backfill_action_item_labels( assert incident.linear_parent_issue_id is not None try: children = service.get_child_issues( - incident.linear_parent_issue_id, include_archived=True + incident.linear_parent_issue_id, + include_archived=True, + raise_on_error=True, + ) + except LinearRateLimitError as error: + stats.errors.insert( + 0, f"{error} Backfill stopped; rerun this selection later." ) + return stats except Exception: logger.exception( "Failed to fetch children for incident %s", incident.incident_number @@ -219,7 +234,14 @@ def backfill_action_item_labels( if dry_run: continue try: - success = service.add_issue_label(issue["id"], label_id) + success = service.add_issue_label( + issue["id"], label_id, raise_on_error=True + ) + except LinearRateLimitError as error: + stats.errors.insert( + 0, f"{error} Backfill stopped; rerun this selection later." + ) + return stats except Exception: logger.exception("Failed to label action item %s", issue["identifier"]) success = False diff --git a/src/firetower/incidents/tests/test_action_item_labels.py b/src/firetower/incidents/tests/test_action_item_labels.py index af4adbca..e3608283 100644 --- a/src/firetower/incidents/tests/test_action_item_labels.py +++ b/src/firetower/incidents/tests/test_action_item_labels.py @@ -16,7 +16,7 @@ backfill_action_item_labels, sync_action_items_from_linear, ) -from firetower.integrations.services.linear import LinearService +from firetower.integrations.services.linear import LinearRateLimitError, LinearService pytestmark = pytest.mark.django_db @@ -153,6 +153,91 @@ def test_does_not_fetch_label_without_children(self, incident, service): class TestBackfillActionItemLabels: + def test_stops_on_rate_limited_label_lookup(self, service): + service.get_issue_label_id.side_effect = LinearRateLimitError("60") + stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) + assert stats.added == 0 + assert "Retry after 60 seconds" in stats.errors[0] + assert "Backfill stopped" in stats.errors[0] + service.get_issue_label_id.assert_called_once_with( + "incident-action", raise_on_error=True + ) + service.get_child_issues.assert_not_called() + service.add_issue_label.assert_not_called() + + @pytest.mark.parametrize("dry_run", [False, True]) + def test_stops_on_rate_limited_child_fetch(self, incident, issue, service, dry_run): + for parent_id in ("parent-2", "parent-3"): + Incident.objects.create( + title=parent_id, + severity=IncidentSeverity.P2, + linear_parent_issue_id=parent_id, + ) + service.get_child_issues.side_effect = [[issue], LinearRateLimitError("60"), []] + stats = backfill_action_item_labels( + Incident.objects.order_by("id"), dry_run=dry_run + ) + assert stats.checked == stats.missing == 1 + assert stats.added == (0 if dry_run else 1) + assert "Backfill stopped" in stats.errors[0] + assert service.get_child_issues.call_count == 2 + assert service.add_issue_label.call_count == (0 if dry_run else 1) + + def test_stops_labeling_on_rate_limit_and_rerun_skips_completed_items( + self, incident, issue, service + ): + Incident.objects.create( + title="Another incident", + severity=IncidentSeverity.P2, + linear_parent_issue_id="parent-2", + ) + children = [ + issue, + {**issue, "id": "issue-2", "identifier": "ENG-2", "label_ids": []}, + {**issue, "id": "issue-3", "identifier": "ENG-3", "label_ids": []}, + ] + service.get_child_issues.return_value = children + service.add_issue_label.side_effect = [True, LinearRateLimitError("90"), True] + stats = backfill_action_item_labels( + Incident.objects.order_by("id"), dry_run=False + ) + assert stats.checked == stats.missing == 2 + assert stats.added == 1 + assert "Retry after 90 seconds" in stats.errors[0] + assert "Backfill stopped" in stats.errors[0] + assert service.add_issue_label.call_count == 2 + service.get_child_issues.assert_called_once_with( + "parent-1", include_archived=True, raise_on_error=True + ) + + issue["label_ids"].append("action-label") + service.add_issue_label.side_effect = None + service.add_issue_label.reset_mock() + stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) + assert stats.checked == 3 + assert stats.missing == stats.added == 2 + assert stats.errors == [] + service.add_issue_label.assert_has_calls( + [ + call("issue-2", "action-label", raise_on_error=True), + call("issue-3", "action-label", raise_on_error=True), + ] + ) + assert service.add_issue_label.call_count == 2 + + def test_rate_limit_message_takes_precedence_over_earlier_failures( + self, incident, issue, service + ): + service.get_child_issues.return_value = [ + issue, + {**issue, "id": "issue-2", "identifier": "ENG-2"}, + ] + service.add_issue_label.side_effect = [False, LinearRateLimitError("60")] + stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) + assert len(stats.errors) == 2 + assert "Backfill stopped" in stats.errors[0] + assert stats.errors[1] == "Failed to label action item ENG-1" + def test_respects_selected_incidents(self, incident, service): Incident.objects.create( title="Not selected", @@ -165,7 +250,7 @@ def test_respects_selected_incidents(self, incident, service): assert stats.checked == stats.missing == stats.added == 1 assert stats.errors == [] service.get_child_issues.assert_called_once_with( - "parent-1", include_archived=True + "parent-1", include_archived=True, raise_on_error=True ) def test_preview_reports_counts_without_adding_labels( @@ -226,7 +311,7 @@ def test_dry_run_does_not_mutate_issues_or_cached_items( ): assert backfill_main([]) == 0 service.get_child_issues.assert_called_once_with( - "parent-1", include_archived=True + "parent-1", include_archived=True, raise_on_error=True ) service.add_issue_label.assert_not_called() service.update_issue.assert_not_called() @@ -244,13 +329,17 @@ def test_apply_skips_existing_labels(self, incident, issue, service, backfill_ma }, ] assert backfill_main(["--apply"]) == 0 - service.add_issue_label.assert_called_once_with("issue-1", "action-label") + service.add_issue_label.assert_called_once_with( + "issue-1", "action-label", raise_on_error=True + ) service.update_issue.assert_not_called() assert incident.action_items.count() == 0 issue["label_ids"].append("action-label") assert backfill_main(["--apply"]) == 0 - service.add_issue_label.assert_called_once_with("issue-1", "action-label") + service.add_issue_label.assert_called_once_with( + "issue-1", "action-label", raise_on_error=True + ) def test_includes_historical_private_incidents_and_deduplicates( self, incident, issue, service, backfill_main @@ -276,13 +365,15 @@ def test_includes_historical_private_incidents_and_deduplicates( service.get_child_issues.assert_has_calls( [ - call("parent-1", include_archived=True), - call("parent-2", include_archived=True), + call("parent-1", include_archived=True, raise_on_error=True), + call("parent-2", include_archived=True, raise_on_error=True), ], any_order=True, ) assert service.get_child_issues.call_count == 2 - service.add_issue_label.assert_called_once_with("issue-1", "action-label") + service.add_issue_label.assert_called_once_with( + "issue-1", "action-label", raise_on_error=True + ) def test_reports_failed_additions(self, incident, service, backfill_main): service.add_issue_label.return_value = False @@ -302,7 +393,9 @@ def test_reports_incomplete_fetch_and_continues( ] assert backfill_main(["--apply"]) == 1 assert service.get_child_issues.call_count == 2 - service.add_issue_label.assert_called_once_with("issue-1", "action-label") + service.add_issue_label.assert_called_once_with( + "issue-1", "action-label", raise_on_error=True + ) def test_stops_when_label_is_missing(self, service, backfill_main): service.get_issue_label_id.return_value = None diff --git a/src/firetower/integrations/services/linear.py b/src/firetower/integrations/services/linear.py index 0223c39d..4deee01c 100644 --- a/src/firetower/integrations/services/linear.py +++ b/src/firetower/integrations/services/linear.py @@ -74,6 +74,16 @@ class LinearError(Exception): """ +class LinearRateLimitError(LinearError): + """Raised for rate-limited requests when the caller opts into errors.""" + + def __init__(self, retry_after: str | None = None) -> None: + message = "Linear API rate limit reached." + if retry_after: + message += f" Retry after {retry_after} seconds." + super().__init__(message) + + # Exact message Linear returns when an ``issue(id:)`` lookup references an # issue that does not exist. Matched case-insensitively. Deliberately specific # to a missing *issue* (not a generic "not found") so unrelated errors @@ -85,6 +95,18 @@ class LinearError(Exception): _MAX_LOGGED_ERRORS_LEN = 2000 +def _errors_are_rate_limited(errors: Any) -> bool: + if not isinstance(errors, list): + return False + for error in errors: + if not isinstance(error, dict): + continue + extensions = error.get("extensions") + if isinstance(extensions, dict) and extensions.get("code") == "RATELIMITED": + return True + return False + + def _summarize_graphql_errors(errors: Any) -> str: """Render a GraphQL ``errors`` payload as a compact string for logging. @@ -260,6 +282,14 @@ def _raise_if_requested(self, message: str, *, raise_on_error: bool) -> None: if raise_on_error: raise LinearError(message) + def _handle_rate_limit( + self, response: requests.Response, *, raise_on_error: bool + ) -> None: + error = LinearRateLimitError(response.headers.get("Retry-After")) + logger.info("%s", error) + if raise_on_error: + raise error + def _graphql( self, query: str, @@ -318,7 +348,20 @@ def _graphql( self._sleep_before_retry(attempt) continue + if response.status_code == 429: + self._handle_rate_limit(response, raise_on_error=raise_on_error) + return None + if not response.ok: + try: + error_data = response.json() + except ValueError: + error_data = None + if isinstance(error_data, dict) and _errors_are_rate_limited( + error_data.get("errors") + ): + self._handle_rate_limit(response, raise_on_error=raise_on_error) + return None logger.error( "Linear API returned %d: %s", response.status_code, @@ -347,6 +390,9 @@ def _graphql( return None if "errors" in data: + if _errors_are_rate_limited(data["errors"]): + self._handle_rate_limit(response, raise_on_error=raise_on_error) + return None # Looking up an issue by an identifier that does not exist # (e.g. the placeholder slot the allocator wants to mint) # comes back as a GraphQL "entity not found" error rather @@ -537,7 +583,9 @@ def create_issue( "url": issue["url"], } - def get_issue_label_id(self, name: str) -> str | None: + def get_issue_label_id( + self, name: str, *, raise_on_error: bool = False + ) -> str | None: """Find an existing workspace-wide label that can be used across teams.""" if name in self._issue_label_ids_cache: return self._issue_label_ids_cache[name] @@ -558,7 +606,7 @@ def get_issue_label_id(self, name: str) -> str | None: } } """ - data = self._graphql(query, {"name": name}) + data = self._graphql(query, {"name": name}, raise_on_error=raise_on_error) if not data: return None labels = data.get("issueLabels", {}).get("nodes", []) @@ -570,7 +618,9 @@ def get_issue_label_id(self, name: str) -> str | None: self._issue_label_ids_cache[name] = label_id return label_id - def add_issue_label(self, issue_id: str, label_id: str) -> bool: + def add_issue_label( + self, issue_id: str, label_id: str, *, raise_on_error: bool = False + ) -> bool: """Add a label atomically without replacing any existing issue labels.""" mutation = """ mutation($id: String!, $labelId: String!) { @@ -579,7 +629,11 @@ def add_issue_label(self, issue_id: str, label_id: str) -> bool: } } """ - data = self._graphql(mutation, {"id": issue_id, "labelId": label_id}) + data = self._graphql( + mutation, + {"id": issue_id, "labelId": label_id}, + raise_on_error=raise_on_error, + ) if not data: return False return data.get("issueAddLabel", {}).get("success", False) @@ -764,7 +818,11 @@ def get_workflow_states(self, team_id: str) -> dict[str, str] | None: return states def get_child_issues( - self, issue_id: str, *, include_archived: bool = False + self, + issue_id: str, + *, + include_archived: bool = False, + raise_on_error: bool = False, ) -> list[dict[str, Any]] | None: query = f""" query($issueId: String!, $after: String, $includeArchived: Boolean!) {{ @@ -794,7 +852,7 @@ def get_child_issues( if cursor is not None: variables["after"] = cursor - data = self._graphql(query, variables) + data = self._graphql(query, variables, raise_on_error=raise_on_error) if data is None: return None diff --git a/src/firetower/integrations/tests/test_linear_service.py b/src/firetower/integrations/tests/test_linear_service.py index b504f48f..1a7d7651 100644 --- a/src/firetower/integrations/tests/test_linear_service.py +++ b/src/firetower/integrations/tests/test_linear_service.py @@ -11,6 +11,7 @@ LINEAR_DEFAULT_TIMEOUT_SECONDS, LINEAR_TOKEN_URL, LinearError, + LinearRateLimitError, LinearService, _errors_are_not_found, _summarize_graphql_errors, @@ -120,6 +121,74 @@ def test_returns_none_on_request_error(self, mock_post, linear_service): class TestGraphql: + @pytest.mark.parametrize("status_code", [200, 400, 429]) + @pytest.mark.parametrize("raise_on_error", [False, True]) + def test_rate_limit_does_not_retry_or_sleep( + self, linear_service, status_code, raise_on_error + ): + response = MagicMock() + response.status_code = status_code + response.ok = status_code < 400 + response.headers = {"Retry-After": "60"} + response.json.return_value = { + "errors": [{"extensions": {"code": "RATELIMITED"}}] + } + with ( + patch.object( + linear_service, "_get_access_token", return_value="valid-token" + ), + patch.object( + linear_service, "_make_graphql_request", return_value=response + ) as request, + patch.object(linear_service, "_sleep_before_retry") as sleep, + ): + if raise_on_error: + with pytest.raises( + LinearRateLimitError, match="Retry after 60 seconds" + ): + linear_service._graphql( + "query { viewer { id } }", raise_on_error=True + ) + else: + assert linear_service._graphql("query { viewer { id } }") is None + + request.assert_called_once() + sleep.assert_not_called() + + def test_handles_429_without_json_body_or_retry_header(self, linear_service): + response = MagicMock() + response.status_code = 429 + response.headers = {} + response.json.side_effect = ValueError("Not JSON") + with ( + patch.object( + linear_service, "_get_access_token", return_value="valid-token" + ), + patch.object( + linear_service, "_make_graphql_request", return_value=response + ), + pytest.raises(LinearRateLimitError, match="Linear API rate limit reached"), + ): + linear_service._graphql("query { viewer { id } }", raise_on_error=True) + response.json.assert_not_called() + + def test_non_rate_limited_http_error_without_json_body(self, linear_service): + response = MagicMock() + response.status_code = 400 + response.ok = False + response.json.side_effect = ValueError("Not JSON") + with ( + patch.object( + linear_service, "_get_access_token", return_value="valid-token" + ), + patch.object( + linear_service, "_make_graphql_request", return_value=response + ), + pytest.raises(LinearError, match="HTTP 400") as error, + ): + linear_service._graphql("query { viewer { id } }", raise_on_error=True) + assert not isinstance(error.value, LinearRateLimitError) + def test_returns_data_on_success(self, linear_service): with ( patch.object( @@ -821,6 +890,22 @@ def test_maps_all_state_types(self, linear_service): class TestIssueLabels: + @pytest.mark.parametrize( + ("method", "args"), + [ + ("get_issue_label_id", ("incident-action",)), + ("get_child_issues", ("parent-1",)), + ("add_issue_label", ("issue-1", "action-label")), + ], + ) + def test_propagates_opt_in_rate_limit_errors(self, linear_service, method, args): + with patch.object( + linear_service, "_graphql", side_effect=LinearRateLimitError("60") + ) as graphql: + with pytest.raises(LinearRateLimitError): + getattr(linear_service, method)(*args, raise_on_error=True) + assert graphql.call_args.kwargs["raise_on_error"] is True + def test_finds_and_caches_workspace_label(self, linear_service): response = {"issueLabels": {"nodes": [{"id": "action-label"}]}} with patch.object(linear_service, "_graphql", return_value=response) as graphql: From 79d760f9d07fc258122bbdb4b3c95b95e2867331 Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Tue, 27 Oct 2026 16:37:40 -0400 Subject: [PATCH 4/7] Remove backfill tests to reduce the pull request --- .../tests/test_action_item_labels.py | 277 +----------------- src/firetower/incidents/tests/test_admin.py | 99 +------ .../integrations/tests/test_linear_service.py | 131 --------- 3 files changed, 3 insertions(+), 504 deletions(-) diff --git a/src/firetower/incidents/tests/test_action_item_labels.py b/src/firetower/incidents/tests/test_action_item_labels.py index e3608283..1a0e33da 100644 --- a/src/firetower/incidents/tests/test_action_item_labels.py +++ b/src/firetower/incidents/tests/test_action_item_labels.py @@ -1,6 +1,4 @@ -import runpy from datetime import timedelta -from pathlib import Path from unittest.mock import MagicMock, call, patch import pytest @@ -12,11 +10,8 @@ IncidentSeverity, IncidentStatus, ) -from firetower.incidents.services import ( - backfill_action_item_labels, - sync_action_items_from_linear, -) -from firetower.integrations.services.linear import LinearRateLimitError, LinearService +from firetower.incidents.services import sync_action_items_from_linear +from firetower.integrations.services.linear import LinearService pytestmark = pytest.mark.django_db @@ -59,16 +54,6 @@ def incident(): ) -@pytest.fixture -def backfill_main(): - script = ( - Path(__file__).resolve().parents[4] - / "scripts/backfill_incident_action_labels.py" - ) - with patch("django.setup"): - yield runpy.run_path(str(script))["main"] - - class TestActionItemLabels: @pytest.mark.parametrize("already_synced", [False, True]) def test_labels_newly_adopted_and_cached_items( @@ -150,261 +135,3 @@ def test_does_not_fetch_label_without_children(self, incident, service): sync_action_items_from_linear(incident, force=True) service.get_issue_label_id.assert_not_called() service.add_issue_label.assert_not_called() - - -class TestBackfillActionItemLabels: - def test_stops_on_rate_limited_label_lookup(self, service): - service.get_issue_label_id.side_effect = LinearRateLimitError("60") - stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) - assert stats.added == 0 - assert "Retry after 60 seconds" in stats.errors[0] - assert "Backfill stopped" in stats.errors[0] - service.get_issue_label_id.assert_called_once_with( - "incident-action", raise_on_error=True - ) - service.get_child_issues.assert_not_called() - service.add_issue_label.assert_not_called() - - @pytest.mark.parametrize("dry_run", [False, True]) - def test_stops_on_rate_limited_child_fetch(self, incident, issue, service, dry_run): - for parent_id in ("parent-2", "parent-3"): - Incident.objects.create( - title=parent_id, - severity=IncidentSeverity.P2, - linear_parent_issue_id=parent_id, - ) - service.get_child_issues.side_effect = [[issue], LinearRateLimitError("60"), []] - stats = backfill_action_item_labels( - Incident.objects.order_by("id"), dry_run=dry_run - ) - assert stats.checked == stats.missing == 1 - assert stats.added == (0 if dry_run else 1) - assert "Backfill stopped" in stats.errors[0] - assert service.get_child_issues.call_count == 2 - assert service.add_issue_label.call_count == (0 if dry_run else 1) - - def test_stops_labeling_on_rate_limit_and_rerun_skips_completed_items( - self, incident, issue, service - ): - Incident.objects.create( - title="Another incident", - severity=IncidentSeverity.P2, - linear_parent_issue_id="parent-2", - ) - children = [ - issue, - {**issue, "id": "issue-2", "identifier": "ENG-2", "label_ids": []}, - {**issue, "id": "issue-3", "identifier": "ENG-3", "label_ids": []}, - ] - service.get_child_issues.return_value = children - service.add_issue_label.side_effect = [True, LinearRateLimitError("90"), True] - stats = backfill_action_item_labels( - Incident.objects.order_by("id"), dry_run=False - ) - assert stats.checked == stats.missing == 2 - assert stats.added == 1 - assert "Retry after 90 seconds" in stats.errors[0] - assert "Backfill stopped" in stats.errors[0] - assert service.add_issue_label.call_count == 2 - service.get_child_issues.assert_called_once_with( - "parent-1", include_archived=True, raise_on_error=True - ) - - issue["label_ids"].append("action-label") - service.add_issue_label.side_effect = None - service.add_issue_label.reset_mock() - stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) - assert stats.checked == 3 - assert stats.missing == stats.added == 2 - assert stats.errors == [] - service.add_issue_label.assert_has_calls( - [ - call("issue-2", "action-label", raise_on_error=True), - call("issue-3", "action-label", raise_on_error=True), - ] - ) - assert service.add_issue_label.call_count == 2 - - def test_rate_limit_message_takes_precedence_over_earlier_failures( - self, incident, issue, service - ): - service.get_child_issues.return_value = [ - issue, - {**issue, "id": "issue-2", "identifier": "ENG-2"}, - ] - service.add_issue_label.side_effect = [False, LinearRateLimitError("60")] - stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) - assert len(stats.errors) == 2 - assert "Backfill stopped" in stats.errors[0] - assert stats.errors[1] == "Failed to label action item ENG-1" - - def test_respects_selected_incidents(self, incident, service): - Incident.objects.create( - title="Not selected", - severity=IncidentSeverity.P2, - linear_parent_issue_id="parent-2", - ) - stats = backfill_action_item_labels( - Incident.objects.filter(pk=incident.pk), dry_run=False - ) - assert stats.checked == stats.missing == stats.added == 1 - assert stats.errors == [] - service.get_child_issues.assert_called_once_with( - "parent-1", include_archived=True, raise_on_error=True - ) - - def test_preview_reports_counts_without_adding_labels( - self, incident, issue, service - ): - service.get_child_issues.return_value = [ - issue, - {**issue, "id": "issue-2", "label_ids": ["action-label"]}, - ] - stats = backfill_action_item_labels(Incident.objects.all()) - assert stats.checked == 2 - assert stats.missing == 1 - assert stats.added == 0 - assert stats.errors == [] - service.add_issue_label.assert_not_called() - - def test_continues_after_unexpected_child_fetch_error(self, incident, service): - Incident.objects.create( - title="Another incident", - severity=IncidentSeverity.P2, - linear_parent_issue_id="parent-2", - ) - service.get_child_issues.side_effect = [ - RuntimeError("Fetch failed"), - service.get_child_issues.return_value, - ] - stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) - assert stats.checked == stats.added == 1 - assert len(stats.errors) == 1 - assert service.get_child_issues.call_count == 2 - - def test_continues_after_unexpected_label_error(self, incident, issue, service): - service.get_child_issues.return_value = [ - issue, - {**issue, "id": "issue-2", "identifier": "ENG-2"}, - ] - service.add_issue_label.side_effect = [RuntimeError("Update failed"), True] - stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) - assert stats.checked == stats.missing == 2 - assert stats.added == 1 - assert stats.errors == ["Failed to label action item ENG-1"] - - def test_reports_label_lookup_exception(self, service): - service.get_issue_label_id.side_effect = RuntimeError("Lookup failed") - stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) - assert stats.errors == ["Failed to fetch Linear incident action label"] - service.get_child_issues.assert_not_called() - service.add_issue_label.assert_not_called() - - def test_reports_missing_configuration(self, settings, service): - settings.LINEAR = None - stats = backfill_action_item_labels(Incident.objects.all(), dry_run=False) - assert stats.errors == ["Linear integration is not configured"] - service.get_issue_label_id.assert_not_called() - - def test_dry_run_does_not_mutate_issues_or_cached_items( - self, incident, service, backfill_main - ): - assert backfill_main([]) == 0 - service.get_child_issues.assert_called_once_with( - "parent-1", include_archived=True, raise_on_error=True - ) - service.add_issue_label.assert_not_called() - service.update_issue.assert_not_called() - service.create_comment.assert_not_called() - assert incident.action_items.count() == 0 - - def test_apply_skips_existing_labels(self, incident, issue, service, backfill_main): - service.get_child_issues.return_value = [ - issue, - { - **issue, - "id": "issue-2", - "identifier": "ENG-2", - "label_ids": ["action-label"], - }, - ] - assert backfill_main(["--apply"]) == 0 - service.add_issue_label.assert_called_once_with( - "issue-1", "action-label", raise_on_error=True - ) - service.update_issue.assert_not_called() - assert incident.action_items.count() == 0 - - issue["label_ids"].append("action-label") - assert backfill_main(["--apply"]) == 0 - service.add_issue_label.assert_called_once_with( - "issue-1", "action-label", raise_on_error=True - ) - - def test_includes_historical_private_incidents_and_deduplicates( - self, incident, issue, service, backfill_main - ): - old_incident = Incident.objects.create( - title="Old private incident", - severity=IncidentSeverity.P4, - status=IncidentStatus.CANCELED, - is_private=True, - linear_parent_issue_id="parent-2", - ) - Incident.objects.filter(pk=old_incident.pk).update( - created_at=timezone.now() - timedelta(days=365) - ) - Incident.objects.create(title="No Linear parent", severity=IncidentSeverity.P2) - Incident.objects.create( - title="Empty Linear parent", - severity=IncidentSeverity.P2, - linear_parent_issue_id="", - ) - - assert backfill_main(["--apply"]) == 0 - - service.get_child_issues.assert_has_calls( - [ - call("parent-1", include_archived=True, raise_on_error=True), - call("parent-2", include_archived=True, raise_on_error=True), - ], - any_order=True, - ) - assert service.get_child_issues.call_count == 2 - service.add_issue_label.assert_called_once_with( - "issue-1", "action-label", raise_on_error=True - ) - - def test_reports_failed_additions(self, incident, service, backfill_main): - service.add_issue_label.return_value = False - assert backfill_main(["--apply"]) == 1 - - def test_reports_incomplete_fetch_and_continues( - self, incident, service, backfill_main - ): - Incident.objects.create( - title="Another incident", - severity=IncidentSeverity.P2, - linear_parent_issue_id="parent-2", - ) - service.get_child_issues.side_effect = [ - None, - service.get_child_issues.return_value, - ] - assert backfill_main(["--apply"]) == 1 - assert service.get_child_issues.call_count == 2 - service.add_issue_label.assert_called_once_with( - "issue-1", "action-label", raise_on_error=True - ) - - def test_stops_when_label_is_missing(self, service, backfill_main): - service.get_issue_label_id.return_value = None - assert backfill_main(["--apply"]) == 1 - service.get_child_issues.assert_not_called() - service.add_issue_label.assert_not_called() - - def test_requires_linear_configuration(self, settings, backfill_main): - settings.LINEAR = None - with pytest.raises(SystemExit) as error: - backfill_main([]) - assert error.value.code == 2 diff --git a/src/firetower/incidents/tests/test_admin.py b/src/firetower/incidents/tests/test_admin.py index e6bcd2ab..dca32b76 100644 --- a/src/firetower/incidents/tests/test_admin.py +++ b/src/firetower/incidents/tests/test_admin.py @@ -1,7 +1,6 @@ from unittest.mock import patch import pytest -from django.contrib import messages from django.contrib.admin.sites import AdminSite from django.contrib.auth.models import User from django.contrib.messages.storage.fallback import FallbackStorage @@ -9,10 +8,7 @@ from firetower.incidents.admin import IncidentAdmin from firetower.incidents.models import Incident, IncidentSeverity, IncidentStatus -from firetower.incidents.services import ( - ActionItemLabelBackfillStats, - ParticipantsSyncStats, -) +from firetower.incidents.services import ParticipantsSyncStats @pytest.mark.django_db @@ -39,99 +35,6 @@ def test_sync_participants_action_exists(self): actions = self.incident_admin.get_actions(request) assert "sync_participants_from_slack" in actions - def test_action_label_actions_exist(self): - request = self._add_session_and_messages(self.factory.get("/")) - actions = self.incident_admin.get_actions(request) - assert "preview_action_item_labels" in actions - assert "backfill_action_item_labels" in actions - - def test_action_label_actions_require_change_permission(self): - request = self._add_session_and_messages(self.factory.get("/")) - request.user = User.objects.create_user(username="read-only", is_staff=True) - actions = self.incident_admin.get_actions(request) - assert "preview_action_item_labels" not in actions - assert "backfill_action_item_labels" not in actions - - @pytest.mark.parametrize( - ("action", "dry_run"), - [("preview_action_item_labels", True), ("backfill_action_item_labels", False)], - ) - def test_action_label_admin_post_limits_backfill_to_selection( - self, client, action, dry_run - ): - selected = Incident.objects.create( - title="Selected", severity=IncidentSeverity.P2 - ) - Incident.objects.create(title="Not selected", severity=IncidentSeverity.P2) - client.force_login(self.admin_user) - - with patch("firetower.incidents.admin.backfill_action_item_labels") as backfill: - backfill.return_value = ActionItemLabelBackfillStats() - response = client.post( - "/admin/incidents/incident/", - {"action": action, "_selected_action": [str(selected.pk)]}, - ) - - assert response.status_code == 302 - backfill.assert_called_once() - assert list(backfill.call_args.args[0]) == [selected] - assert backfill.call_args.kwargs == {"dry_run": dry_run} - - @pytest.mark.parametrize( - ("action", "dry_run"), - [("preview_action_item_labels", True), ("backfill_action_item_labels", False)], - ) - def test_action_label_actions_report_counts(self, action, dry_run): - request = self._add_session_and_messages(self.factory.post("/")) - queryset = Incident.objects.none() - with ( - patch("firetower.incidents.admin.backfill_action_item_labels") as backfill, - patch.object(self.incident_admin, "message_user") as message_user, - ): - backfill.return_value = ActionItemLabelBackfillStats( - checked=3, missing=2, added=0 if dry_run else 2 - ) - getattr(self.incident_admin, action)(request, queryset) - - backfill.assert_called_once_with(queryset, dry_run=dry_run) - message = message_user.call_args.args[1] - assert "3 checked" in message - assert "2 missing labels" in message - assert f"{0 if dry_run else 2} added" in message - assert "0 failures" in message - assert message_user.call_args.kwargs["level"] == messages.SUCCESS - - def test_action_label_backfill_reports_partial_failure(self): - request = self._add_session_and_messages(self.factory.post("/")) - with ( - patch("firetower.incidents.admin.backfill_action_item_labels") as backfill, - patch.object(self.incident_admin, "message_user") as message_user, - ): - backfill.return_value = ActionItemLabelBackfillStats( - checked=3, - missing=2, - added=1, - errors=["Failed to label action item ENG-1"], - ) - self.incident_admin.backfill_action_item_labels( - request, Incident.objects.none() - ) - - message = message_user.call_args.args[1] - assert "1 added" in message - assert "1 failures" in message - assert "Failed to label action item ENG-1" in message - assert message_user.call_args.kwargs["level"] == messages.ERROR - - def test_action_label_backfill_reports_missing_configuration(self): - request = self._add_session_and_messages(self.factory.post("/")) - with patch.object(self.incident_admin, "message_user") as message_user: - self.incident_admin.backfill_action_item_labels( - request, Incident.objects.none() - ) - assert "Linear integration is not configured" in message_user.call_args.args[1] - assert message_user.call_args.kwargs["level"] == messages.ERROR - def test_sync_participants_action_calls_sync_function(self): incident1 = Incident.objects.create( title="Test Incident 1", diff --git a/src/firetower/integrations/tests/test_linear_service.py b/src/firetower/integrations/tests/test_linear_service.py index 1a7d7651..9a7096b3 100644 --- a/src/firetower/integrations/tests/test_linear_service.py +++ b/src/firetower/integrations/tests/test_linear_service.py @@ -11,7 +11,6 @@ LINEAR_DEFAULT_TIMEOUT_SECONDS, LINEAR_TOKEN_URL, LinearError, - LinearRateLimitError, LinearService, _errors_are_not_found, _summarize_graphql_errors, @@ -121,74 +120,6 @@ def test_returns_none_on_request_error(self, mock_post, linear_service): class TestGraphql: - @pytest.mark.parametrize("status_code", [200, 400, 429]) - @pytest.mark.parametrize("raise_on_error", [False, True]) - def test_rate_limit_does_not_retry_or_sleep( - self, linear_service, status_code, raise_on_error - ): - response = MagicMock() - response.status_code = status_code - response.ok = status_code < 400 - response.headers = {"Retry-After": "60"} - response.json.return_value = { - "errors": [{"extensions": {"code": "RATELIMITED"}}] - } - with ( - patch.object( - linear_service, "_get_access_token", return_value="valid-token" - ), - patch.object( - linear_service, "_make_graphql_request", return_value=response - ) as request, - patch.object(linear_service, "_sleep_before_retry") as sleep, - ): - if raise_on_error: - with pytest.raises( - LinearRateLimitError, match="Retry after 60 seconds" - ): - linear_service._graphql( - "query { viewer { id } }", raise_on_error=True - ) - else: - assert linear_service._graphql("query { viewer { id } }") is None - - request.assert_called_once() - sleep.assert_not_called() - - def test_handles_429_without_json_body_or_retry_header(self, linear_service): - response = MagicMock() - response.status_code = 429 - response.headers = {} - response.json.side_effect = ValueError("Not JSON") - with ( - patch.object( - linear_service, "_get_access_token", return_value="valid-token" - ), - patch.object( - linear_service, "_make_graphql_request", return_value=response - ), - pytest.raises(LinearRateLimitError, match="Linear API rate limit reached"), - ): - linear_service._graphql("query { viewer { id } }", raise_on_error=True) - response.json.assert_not_called() - - def test_non_rate_limited_http_error_without_json_body(self, linear_service): - response = MagicMock() - response.status_code = 400 - response.ok = False - response.json.side_effect = ValueError("Not JSON") - with ( - patch.object( - linear_service, "_get_access_token", return_value="valid-token" - ), - patch.object( - linear_service, "_make_graphql_request", return_value=response - ), - pytest.raises(LinearError, match="HTTP 400") as error, - ): - linear_service._graphql("query { viewer { id } }", raise_on_error=True) - assert not isinstance(error.value, LinearRateLimitError) - def test_returns_data_on_success(self, linear_service): with ( patch.object( @@ -890,22 +821,6 @@ def test_maps_all_state_types(self, linear_service): class TestIssueLabels: - @pytest.mark.parametrize( - ("method", "args"), - [ - ("get_issue_label_id", ("incident-action",)), - ("get_child_issues", ("parent-1",)), - ("add_issue_label", ("issue-1", "action-label")), - ], - ) - def test_propagates_opt_in_rate_limit_errors(self, linear_service, method, args): - with patch.object( - linear_service, "_graphql", side_effect=LinearRateLimitError("60") - ) as graphql: - with pytest.raises(LinearRateLimitError): - getattr(linear_service, method)(*args, raise_on_error=True) - assert graphql.call_args.kwargs["raise_on_error"] is True - def test_finds_and_caches_workspace_label(self, linear_service): response = {"issueLabels": {"nodes": [{"id": "action-label"}]}} with patch.object(linear_service, "_graphql", return_value=response) as graphql: @@ -955,52 +870,6 @@ def test_reports_addition_failure(self, linear_service, response): class TestGetChildIssues: - def test_includes_archived_children_when_requested(self, linear_service): - response = { - "issue": {"children": {"nodes": [], "pageInfo": {"hasNextPage": False}}} - } - with patch.object(linear_service, "_graphql", return_value=response) as graphql: - assert ( - linear_service.get_child_issues("parent-id", include_archived=True) - == [] - ) - assert graphql.call_args.args[1]["includeArchived"] is True - assert "includeArchived: $includeArchived" in graphql.call_args.args[0] - - @pytest.mark.parametrize("end_cursor", [None, "cursor1"]) - def test_rejects_incomplete_pagination(self, linear_service, end_cursor): - def page(cursor): - return { - "issue": { - "children": { - "nodes": [], - "pageInfo": {"hasNextPage": True, "endCursor": cursor}, - } - } - } - - with patch.object( - linear_service, "_graphql", side_effect=[page("cursor1"), page(end_cursor)] - ): - assert linear_service.get_child_issues("parent-id") is None - - def test_rejects_page_limit_instead_of_returning_partial_children( - self, linear_service - ): - pages = [ - { - "issue": { - "children": { - "nodes": [], - "pageInfo": {"hasNextPage": True, "endCursor": str(index)}, - } - } - } - for index in range(25) - ] - with patch.object(linear_service, "_graphql", side_effect=pages): - assert linear_service.get_child_issues("parent-id") is None - def test_paginates_through_multiple_pages(self, linear_service): with patch.object( linear_service, From 3a2e7b804efc46b59f26c6c1e50a2761b5ed3f7e Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Tue, 27 Oct 2026 16:51:22 -0400 Subject: [PATCH 5/7] Remove backfill tooling and keep automatic action labels --- README.md | 12 -- scripts/backfill_incident_action_labels.py | 36 ------ src/firetower/incidents/admin.py | 37 +----- src/firetower/incidents/services.py | 115 +----------------- src/firetower/integrations/services/linear.py | 96 +++------------ 5 files changed, 16 insertions(+), 280 deletions(-) delete mode 100644 scripts/backfill_incident_action_labels.py diff --git a/README.md b/README.md index fec756e7..26c9d4e8 100644 --- a/README.md +++ b/README.md @@ -131,18 +131,6 @@ sdk/ # Python SDK Action item sync adds the existing workspace-wide `incident-action` label to incident sub-issues, including pre-existing issues adopted as follow-ups. Other labels are preserved. -To backfill historical action items in production, open **Django admin → Incidents**: - -1. Select a batch of incidents. -2. Run **Preview incident-action label backfill** to see how many labels are missing without updating Linear issues. -3. Select the same incidents and run **Backfill incident-action labels in Linear** to apply the additions. - -Both actions require permission to change incidents and report counts and failures in admin. They read current sub-issues for the selected incident parents, including archived children, without changing parent relationships, SLAs, or cached action items. Use manageable batches to keep admin requests short; rerunning safely skips existing labels. The workspace label must already exist. - -If Linear rate-limits a request, the action stops immediately and reports partial progress and any `Retry-After` guidance. It does not wait for quota to reset inside the admin request or resume automatically. Wait for the limit to reset, then rerun the same selection; issues already labeled are skipped. - -The optional local script, `scripts/backfill_incident_action_labels.py`, uses the same backfill logic and still defaults to a dry run. - ## Scheduled Tasks Scheduled tasks are stored as database objects and are managed via migrations. diff --git a/scripts/backfill_incident_action_labels.py b/scripts/backfill_incident_action_labels.py deleted file mode 100644 index 0aa4462f..00000000 --- a/scripts/backfill_incident_action_labels.py +++ /dev/null @@ -1,36 +0,0 @@ -"""Backfill incident-action labels without changing issues' other fields. - -Preview: uv run python scripts/backfill_incident_action_labels.py -Apply: uv run python scripts/backfill_incident_action_labels.py --apply -""" - -import argparse -import logging -import os - -import django -from django.conf import settings - - -def main(argv: list[str] | None = None) -> int: - parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("--apply", action="store_true", help="Apply label additions") - args = parser.parse_args(argv) - - os.environ.setdefault("DJANGO_SETTINGS_MODULE", "firetower.settings") - django.setup() - - from firetower.incidents.models import Incident # noqa: PLC0415 - from firetower.incidents.services import backfill_action_item_labels # noqa: PLC0415 - - if not settings.LINEAR: - parser.error("Linear integration is not configured") - - stats = backfill_action_item_labels(Incident.objects.all(), dry_run=not args.apply) - for error in stats.errors: - logging.getLogger(__name__).error("%s", error) - return 1 if stats.errors else 0 - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/src/firetower/incidents/admin.py b/src/firetower/incidents/admin.py index 65249323..0db98a2c 100644 --- a/src/firetower/incidents/admin.py +++ b/src/firetower/incidents/admin.py @@ -1,12 +1,11 @@ from __future__ import annotations -from django.contrib import admin, messages +from django.contrib import admin from django.db.models import QuerySet from django.http import HttpRequest from .models import ExternalLink, Incident, Tag from .services import ( - backfill_action_item_labels, sync_action_items_from_linear, sync_incident_participants_from_slack, ) @@ -43,8 +42,6 @@ class IncidentAdmin(admin.ModelAdmin): actions = [ "sync_participants_from_slack", "sync_action_items", - "preview_action_item_labels", - "backfill_action_item_labels", "clear_milestones", ] @@ -152,38 +149,6 @@ def sync_action_items( self.message_user(request, f"Action item sync: {', '.join(message_parts)}") - @admin.action( - description="Preview incident-action label backfill", permissions=["change"] - ) - def preview_action_item_labels( - self, request: HttpRequest, queryset: QuerySet[Incident] - ) -> None: - self._backfill_action_item_labels(request, queryset, dry_run=True) - - @admin.action( - description="Backfill incident-action labels in Linear", permissions=["change"] - ) - def backfill_action_item_labels( - self, request: HttpRequest, queryset: QuerySet[Incident] - ) -> None: - self._backfill_action_item_labels(request, queryset, dry_run=False) - - def _backfill_action_item_labels( - self, request: HttpRequest, queryset: QuerySet[Incident], *, dry_run: bool - ) -> None: - stats = backfill_action_item_labels(queryset, dry_run=dry_run) - operation = "preview" if dry_run else "backfill" - message = ( - f"Action label {operation}: {stats.checked} checked, " - f"{stats.missing} missing labels, {stats.added} added, " - f"{len(stats.errors)} failures." - ) - if stats.errors: - message += f" {stats.errors[0]}" - self.message_user( - request, message, level=messages.ERROR if stats.errors else messages.SUCCESS - ) - @admin.action(description="Clear all milestones") def clear_milestones( self, request: HttpRequest, queryset: QuerySet[Incident] diff --git a/src/firetower/incidents/services.py b/src/firetower/incidents/services.py index d7a5b564..e20f7797 100644 --- a/src/firetower/incidents/services.py +++ b/src/firetower/incidents/services.py @@ -5,7 +5,6 @@ from django.conf import settings from django.contrib.auth.models import User from django.db import transaction -from django.db.models import QuerySet from django.utils import timezone from firetower.auth.models import ExternalProfile, ExternalProfileType @@ -22,10 +21,7 @@ IncidentStatus, ) from firetower.integrations.services import LinearService, SlackService -from firetower.integrations.services.linear import ( - INCIDENT_ACTION_LABEL_NAME, - LinearRateLimitError, -) +from firetower.integrations.services.linear import INCIDENT_ACTION_LABEL_NAME logger = logging.getLogger(__name__) _slack_service = SlackService() @@ -154,115 +150,6 @@ class ActionItemsSyncStats: skipped: bool = False -@dataclass -class ActionItemLabelBackfillStats: - checked: int = 0 - missing: int = 0 - added: int = 0 - errors: list[str] = field(default_factory=list) - - -def backfill_action_item_labels( - incidents: QuerySet[Incident], *, dry_run: bool = True -) -> ActionItemLabelBackfillStats: - """Label live incident sub-issues without running other sync side effects.""" - stats = ActionItemLabelBackfillStats() - if not settings.LINEAR: - stats.errors.append("Linear integration is not configured") - return stats - - try: - service = _get_linear_service() - label_id = service.get_issue_label_id( - INCIDENT_ACTION_LABEL_NAME, raise_on_error=True - ) - except LinearRateLimitError as error: - stats.errors.insert(0, f"{error} Backfill stopped; rerun this selection later.") - return stats - except Exception: - logger.exception("Failed to fetch Linear incident action label") - stats.errors.append("Failed to fetch Linear incident action label") - return stats - if not label_id: - stats.errors.append( - f"Linear workspace label {INCIDENT_ACTION_LABEL_NAME} is unavailable" - ) - return stats - - incidents = incidents.exclude(linear_parent_issue_id__isnull=True).exclude( - linear_parent_issue_id="" - ) - seen: set[str] = set() - for incident in incidents.iterator(): - assert incident.linear_parent_issue_id is not None - try: - children = service.get_child_issues( - incident.linear_parent_issue_id, - include_archived=True, - raise_on_error=True, - ) - except LinearRateLimitError as error: - stats.errors.insert( - 0, f"{error} Backfill stopped; rerun this selection later." - ) - return stats - except Exception: - logger.exception( - "Failed to fetch children for incident %s", incident.incident_number - ) - children = None - if children is None: - stats.errors.append( - f"Failed to fetch children for incident {incident.incident_number}" - ) - continue - - for issue in children: - if issue["id"] in seen: - continue - seen.add(issue["id"]) - stats.checked += 1 - if label_id in issue.get("label_ids", []): - continue - stats.missing += 1 - logger.info( - "%s %s to %s", - "Would add" if dry_run else "Adding", - INCIDENT_ACTION_LABEL_NAME, - issue["identifier"], - ) - if dry_run: - continue - try: - success = service.add_issue_label( - issue["id"], label_id, raise_on_error=True - ) - except LinearRateLimitError as error: - stats.errors.insert( - 0, f"{error} Backfill stopped; rerun this selection later." - ) - return stats - except Exception: - logger.exception("Failed to label action item %s", issue["identifier"]) - success = False - if success: - stats.added += 1 - else: - stats.errors.append( - f"Failed to label action item {issue['identifier']}" - ) - - logger.info( - "%s: %d checked, %d missing labels, %d added, %d failures", - "Dry run" if dry_run else "Backfill", - stats.checked, - stats.missing, - stats.added, - len(stats.errors), - ) - return stats - - def _get_action_item_slo_days(priority: int) -> int | None: linear = settings.LINEAR or {} if priority in (1, 2): diff --git a/src/firetower/integrations/services/linear.py b/src/firetower/integrations/services/linear.py index 4deee01c..662dbe3e 100644 --- a/src/firetower/integrations/services/linear.py +++ b/src/firetower/integrations/services/linear.py @@ -74,16 +74,6 @@ class LinearError(Exception): """ -class LinearRateLimitError(LinearError): - """Raised for rate-limited requests when the caller opts into errors.""" - - def __init__(self, retry_after: str | None = None) -> None: - message = "Linear API rate limit reached." - if retry_after: - message += f" Retry after {retry_after} seconds." - super().__init__(message) - - # Exact message Linear returns when an ``issue(id:)`` lookup references an # issue that does not exist. Matched case-insensitively. Deliberately specific # to a missing *issue* (not a generic "not found") so unrelated errors @@ -95,18 +85,6 @@ def __init__(self, retry_after: str | None = None) -> None: _MAX_LOGGED_ERRORS_LEN = 2000 -def _errors_are_rate_limited(errors: Any) -> bool: - if not isinstance(errors, list): - return False - for error in errors: - if not isinstance(error, dict): - continue - extensions = error.get("extensions") - if isinstance(extensions, dict) and extensions.get("code") == "RATELIMITED": - return True - return False - - def _summarize_graphql_errors(errors: Any) -> str: """Render a GraphQL ``errors`` payload as a compact string for logging. @@ -282,14 +260,6 @@ def _raise_if_requested(self, message: str, *, raise_on_error: bool) -> None: if raise_on_error: raise LinearError(message) - def _handle_rate_limit( - self, response: requests.Response, *, raise_on_error: bool - ) -> None: - error = LinearRateLimitError(response.headers.get("Retry-After")) - logger.info("%s", error) - if raise_on_error: - raise error - def _graphql( self, query: str, @@ -348,20 +318,7 @@ def _graphql( self._sleep_before_retry(attempt) continue - if response.status_code == 429: - self._handle_rate_limit(response, raise_on_error=raise_on_error) - return None - if not response.ok: - try: - error_data = response.json() - except ValueError: - error_data = None - if isinstance(error_data, dict) and _errors_are_rate_limited( - error_data.get("errors") - ): - self._handle_rate_limit(response, raise_on_error=raise_on_error) - return None logger.error( "Linear API returned %d: %s", response.status_code, @@ -390,9 +347,6 @@ def _graphql( return None if "errors" in data: - if _errors_are_rate_limited(data["errors"]): - self._handle_rate_limit(response, raise_on_error=raise_on_error) - return None # Looking up an issue by an identifier that does not exist # (e.g. the placeholder slot the allocator wants to mint) # comes back as a GraphQL "entity not found" error rather @@ -583,9 +537,7 @@ def create_issue( "url": issue["url"], } - def get_issue_label_id( - self, name: str, *, raise_on_error: bool = False - ) -> str | None: + def get_issue_label_id(self, name: str) -> str | None: """Find an existing workspace-wide label that can be used across teams.""" if name in self._issue_label_ids_cache: return self._issue_label_ids_cache[name] @@ -606,7 +558,7 @@ def get_issue_label_id( } } """ - data = self._graphql(query, {"name": name}, raise_on_error=raise_on_error) + data = self._graphql(query, {"name": name}) if not data: return None labels = data.get("issueLabels", {}).get("nodes", []) @@ -618,9 +570,7 @@ def get_issue_label_id( self._issue_label_ids_cache[name] = label_id return label_id - def add_issue_label( - self, issue_id: str, label_id: str, *, raise_on_error: bool = False - ) -> bool: + def add_issue_label(self, issue_id: str, label_id: str) -> bool: """Add a label atomically without replacing any existing issue labels.""" mutation = """ mutation($id: String!, $labelId: String!) { @@ -629,11 +579,7 @@ def add_issue_label( } } """ - data = self._graphql( - mutation, - {"id": issue_id, "labelId": label_id}, - raise_on_error=raise_on_error, - ) + data = self._graphql(mutation, {"id": issue_id, "labelId": label_id}) if not data: return False return data.get("issueAddLabel", {}).get("success", False) @@ -817,17 +763,11 @@ def get_workflow_states(self, team_id: str) -> dict[str, str] | None: self._workflow_states_cache = states return states - def get_child_issues( - self, - issue_id: str, - *, - include_archived: bool = False, - raise_on_error: bool = False, - ) -> list[dict[str, Any]] | None: + def get_child_issues(self, issue_id: str) -> list[dict[str, Any]] | None: query = f""" - query($issueId: String!, $after: String, $includeArchived: Boolean!) {{ + query($issueId: String!, $after: String) {{ issue(id: $issueId) {{ - children(first: 50, after: $after, includeArchived: $includeArchived) {{ + children(first: 50, after: $after) {{ nodes {{ {ISSUE_FIELDS} }} @@ -845,14 +785,11 @@ def get_child_issues( max_pages = 25 for _ in range(max_pages): - variables: dict[str, Any] = { - "issueId": issue_id, - "includeArchived": include_archived, - } + variables: dict[str, Any] = {"issueId": issue_id} if cursor is not None: variables["after"] = cursor - data = self._graphql(query, variables, raise_on_error=raise_on_error) + data = self._graphql(query, variables) if data is None: return None @@ -867,14 +804,9 @@ def get_child_issues( page_info = children.get("pageInfo", {}) if not page_info.get("hasNextPage"): - return issues - next_cursor = page_info.get("endCursor") - if not next_cursor or next_cursor == cursor: - logger.warning( - "Incomplete Linear child issue pagination for %s", issue_id - ) - return None - cursor = next_cursor + break + cursor = page_info.get("endCursor") + if cursor is None: + break - logger.warning("Reached Linear child issue page limit for %s", issue_id) - return None + return issues From 5eb13ab50c177a2376ba66575532698cf00f4602 Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Wed, 28 Oct 2026 12:26:14 -0400 Subject: [PATCH 6/7] Configure incident action label ID through an environment variable --- README.md | 2 +- src/firetower/incidents/services.py | 14 ++------ .../tests/test_action_item_labels.py | 20 +++++------ src/firetower/integrations/services/linear.py | 35 ------------------- .../integrations/tests/test_linear_service.py | 30 ---------------- src/firetower/settings.py | 1 + 6 files changed, 12 insertions(+), 90 deletions(-) diff --git a/README.md b/README.md index 26c9d4e8..985e0e78 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,7 @@ sdk/ # Python SDK ## Incident Action Labels -Action item sync adds the existing workspace-wide `incident-action` label to incident sub-issues, including pre-existing issues adopted as follow-ups. Other labels are preserved. +Set `LINEAR_ACTION_ITEM_LABEL_ID` to the workspace-wide `incident-action` label's ID to enable automatic labeling during action item sync, including pre-existing issues adopted as follow-ups. Other labels are preserved. Labeling is skipped when the environment variable is unset. ## Scheduled Tasks diff --git a/src/firetower/incidents/services.py b/src/firetower/incidents/services.py index e20f7797..cb33df28 100644 --- a/src/firetower/incidents/services.py +++ b/src/firetower/incidents/services.py @@ -21,7 +21,6 @@ IncidentStatus, ) from firetower.integrations.services import LinearService, SlackService -from firetower.integrations.services.linear import INCIDENT_ACTION_LABEL_NAME logger = logging.getLogger(__name__) _slack_service = SlackService() @@ -190,19 +189,10 @@ def _set_action_item_sla(linear_service: LinearService, issue: dict) -> None: def _set_action_item_labels( linear_service: LinearService, issues: list[dict] ) -> list[str]: - if not issues: + label_id = (settings.LINEAR or {}).get("ACTION_ITEM_LABEL_ID") + if not issues or not label_id: return [] - try: - label_id = linear_service.get_issue_label_id(INCIDENT_ACTION_LABEL_NAME) - except Exception: - logger.exception("Failed to fetch Linear incident action label") - return ["Failed to fetch Linear incident action label"] - if not label_id: - error = f"Linear workspace label {INCIDENT_ACTION_LABEL_NAME} is unavailable" - logger.warning(error) - return [error] - errors = [] for issue in issues: if label_id in issue.get("label_ids", []): diff --git a/src/firetower/incidents/tests/test_action_item_labels.py b/src/firetower/incidents/tests/test_action_item_labels.py index 1a0e33da..b8b75dda 100644 --- a/src/firetower/incidents/tests/test_action_item_labels.py +++ b/src/firetower/incidents/tests/test_action_item_labels.py @@ -33,10 +33,12 @@ def issue(): @pytest.fixture def service(settings, issue): - settings.LINEAR = {"TEAM_ID": "incident-team"} + settings.LINEAR = { + "TEAM_ID": "incident-team", + "ACTION_ITEM_LABEL_ID": "action-label", + } service = MagicMock(spec=LinearService) service.get_child_issues.return_value = [issue] - service.get_issue_label_id.return_value = "action-label" service.add_issue_label.return_value = True with patch( "firetower.incidents.services._get_linear_service", return_value=service @@ -74,7 +76,6 @@ def test_labels_newly_adopted_and_cached_items( item = incident.action_items.get() assert item.created_at == issue["created_at"] service.add_issue_label.assert_called_once_with("issue-1", "action-label") - service.get_issue_label_id.assert_called_once_with("incident-action") @pytest.mark.parametrize("severity", IncidentSeverity.values) @pytest.mark.parametrize("status", ["Todo", "In Progress", "Done", "Canceled"]) @@ -117,21 +118,16 @@ def test_label_failure_does_not_block_sync_or_other_items( assert stats.errors == [] assert stats.updated == 2 - @pytest.mark.parametrize("failure", [None, RuntimeError("Label lookup failed")]) - def test_missing_label_does_not_block_sync(self, incident, service, failure): - if isinstance(failure, Exception): - service.get_issue_label_id.side_effect = failure - else: - service.get_issue_label_id.return_value = failure + def test_skips_labeling_without_configured_id(self, settings, incident, service): + settings.LINEAR.pop("ACTION_ITEM_LABEL_ID") stats = sync_action_items_from_linear(incident, force=True) assert stats.created == 1 - assert len(stats.errors) == 1 + assert stats.errors == [] service.add_issue_label.assert_not_called() - def test_does_not_fetch_label_without_children(self, incident, service): + def test_does_not_label_without_children(self, incident, service): service.get_child_issues.return_value = [] sync_action_items_from_linear(incident, force=True) - service.get_issue_label_id.assert_not_called() service.add_issue_label.assert_not_called() diff --git a/src/firetower/integrations/services/linear.py b/src/firetower/integrations/services/linear.py index 662dbe3e..34d35980 100644 --- a/src/firetower/integrations/services/linear.py +++ b/src/firetower/integrations/services/linear.py @@ -16,7 +16,6 @@ LINEAR_API_URL = "https://api.linear.app/graphql" LINEAR_TOKEN_URL = "https://api.linear.app/oauth/token" -INCIDENT_ACTION_LABEL_NAME = "incident-action" LINEAR_STATE_TYPE_MAP = { "triage": "Todo", @@ -169,7 +168,6 @@ def __init__( timeout if timeout is not None else LINEAR_DEFAULT_TIMEOUT_SECONDS ) self._workflow_states_cache: dict[str, str] | None = None - self._issue_label_ids_cache: dict[str, str] = {} @classmethod def for_allocation(cls) -> "LinearService": @@ -537,39 +535,6 @@ def create_issue( "url": issue["url"], } - def get_issue_label_id(self, name: str) -> str | None: - """Find an existing workspace-wide label that can be used across teams.""" - if name in self._issue_label_ids_cache: - return self._issue_label_ids_cache[name] - - query = """ - query($name: String!) { - issueLabels( - filter: { - name: { eq: $name }, - team: { null: true }, - isGroup: { eq: false } - }, - first: 2 - ) { - nodes { - id - } - } - } - """ - data = self._graphql(query, {"name": name}) - if not data: - return None - labels = data.get("issueLabels", {}).get("nodes", []) - if len(labels) != 1: - logger.warning("Expected one Linear workspace label named %s", name) - return None - - label_id = labels[0]["id"] - self._issue_label_ids_cache[name] = label_id - return label_id - def add_issue_label(self, issue_id: str, label_id: str) -> bool: """Add a label atomically without replacing any existing issue labels.""" mutation = """ diff --git a/src/firetower/integrations/tests/test_linear_service.py b/src/firetower/integrations/tests/test_linear_service.py index 9a7096b3..fbbb4e99 100644 --- a/src/firetower/integrations/tests/test_linear_service.py +++ b/src/firetower/integrations/tests/test_linear_service.py @@ -821,36 +821,6 @@ def test_maps_all_state_types(self, linear_service): class TestIssueLabels: - def test_finds_and_caches_workspace_label(self, linear_service): - response = {"issueLabels": {"nodes": [{"id": "action-label"}]}} - with patch.object(linear_service, "_graphql", return_value=response) as graphql: - assert ( - linear_service.get_issue_label_id("incident-action") == "action-label" - ) - assert ( - linear_service.get_issue_label_id("incident-action") == "action-label" - ) - - graphql.assert_called_once() - query, variables = graphql.call_args.args - assert variables == {"name": "incident-action"} - assert "team: { null: true }" in query - assert "isGroup: { eq: false }" in query - - @pytest.mark.parametrize( - "response", - [ - None, - {"issueLabels": {"nodes": []}}, - {"issueLabels": {"nodes": [{"id": "a"}, {"id": "b"}]}}, - ], - ) - def test_does_not_cache_missing_or_ambiguous_label(self, linear_service, response): - with patch.object(linear_service, "_graphql", return_value=response) as graphql: - assert linear_service.get_issue_label_id("incident-action") is None - assert linear_service.get_issue_label_id("incident-action") is None - assert graphql.call_count == 2 - def test_adds_label_without_replacing_existing_labels(self, linear_service): with patch.object( linear_service, diff --git a/src/firetower/settings.py b/src/firetower/settings.py index 7315d51b..f0a4536d 100644 --- a/src/firetower/settings.py +++ b/src/firetower/settings.py @@ -346,6 +346,7 @@ class StatuspageSettings(TypedDict): "API_KEY": config.linear.api_key, "TEAM_ID": config.linear.team_id, "PROJECT_ID": config.linear.project_id, + "ACTION_ITEM_LABEL_ID": os.environ.get("LINEAR_ACTION_ITEM_LABEL_ID", ""), "SYNC_IDENTIFIERS": config.linear.sync_identifiers, "INCIDENT_ADOPT_ON_CREATE": config.linear.adopt_on_create, "ACTION_ITEM_SLO_DAYS_HIGH_PRIORITY": config.linear.action_item_slo_days_high_priority, From 8f5e5897d68334e043f2ddb7736e24af2fab9dc1 Mon Sep 17 00:00:00 2001 From: Spencer Murray Date: Wed, 28 Oct 2026 12:30:50 -0400 Subject: [PATCH 7/7] Read the incident action label ID from the config file --- README.md | 2 +- config.example.toml | 1 + src/firetower/config.py | 1 + src/firetower/settings.py | 2 +- 4 files changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 985e0e78..48811297 100644 --- a/README.md +++ b/README.md @@ -129,7 +129,7 @@ sdk/ # Python SDK ## Incident Action Labels -Set `LINEAR_ACTION_ITEM_LABEL_ID` to the workspace-wide `incident-action` label's ID to enable automatic labeling during action item sync, including pre-existing issues adopted as follow-ups. Other labels are preserved. Labeling is skipped when the environment variable is unset. +Set `action_item_label_id` in the `[linear]` section of `config.toml` to the workspace-wide `incident-action` label's ID to enable automatic labeling during action item sync, including pre-existing issues adopted as follow-ups. Other labels are preserved. Labeling is skipped when the value is empty or omitted. ## Scheduled Tasks diff --git a/config.example.toml b/config.example.toml index f1575ef9..25568dd3 100644 --- a/config.example.toml +++ b/config.example.toml @@ -86,6 +86,7 @@ client_secret = "" # Overridable via LINEAR_CLIENT_SECRET action_item_sync_throttle_seconds = 300 team_id = "" project_id = "" +action_item_label_id = "" # When true, claim the Linear issue matching the incident number (e.g. INC-2160) instead of creating a new one. sync_identifiers = false # When true (and sync_identifiers is true), allocate incident ids by claiming or diff --git a/src/firetower/config.py b/src/firetower/config.py index 38ff8342..536d4e0e 100644 --- a/src/firetower/config.py +++ b/src/firetower/config.py @@ -109,6 +109,7 @@ class LinearConfig: action_item_sync_throttle_seconds: int = 300 team_id: str = "" project_id: str = "" + action_item_label_id: str = "" sync_identifiers: bool = False adopt_on_create: bool = False api_key: str = "" diff --git a/src/firetower/settings.py b/src/firetower/settings.py index f0a4536d..a71d0ab2 100644 --- a/src/firetower/settings.py +++ b/src/firetower/settings.py @@ -346,7 +346,7 @@ class StatuspageSettings(TypedDict): "API_KEY": config.linear.api_key, "TEAM_ID": config.linear.team_id, "PROJECT_ID": config.linear.project_id, - "ACTION_ITEM_LABEL_ID": os.environ.get("LINEAR_ACTION_ITEM_LABEL_ID", ""), + "ACTION_ITEM_LABEL_ID": config.linear.action_item_label_id, "SYNC_IDENTIFIERS": config.linear.sync_identifiers, "INCIDENT_ADOPT_ON_CREATE": config.linear.adopt_on_create, "ACTION_ITEM_SLO_DAYS_HIGH_PRIORITY": config.linear.action_item_slo_days_high_priority,