-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat(seer): Add helper for bulk updating Seer project settings #115756
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9085730
997694d
184eeb2
df76e57
83974d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -733,73 +733,126 @@ class SeerProjectSettingsUpdate(TypedDict, total=False): | |
| scannerAutomation: bool | ||
|
|
||
|
|
||
| def _get_seer_project_options_to_update( | ||
| data: SeerProjectSettingsUpdate, | ||
| ) -> tuple[dict[str, Any], list[str]]: | ||
| """Return (options_to_set, options_to_clear) for the given Seer project settings update. | ||
| Clear the option if it's the default; otherwise, set it.""" | ||
| options_to_set: dict[str, Any] = {} | ||
| options_to_clear: list[str] = [] | ||
|
|
||
| def _set_or_clear(key: str, value: Any, default: Any) -> None: | ||
| if value == default: | ||
| options_to_clear.append(key) | ||
| else: | ||
| options_to_set[key] = value | ||
|
|
||
| if "agent" in data: | ||
| agent = data["agent"] | ||
| if agent == AutomationCodingAgent.SEER: | ||
| options_to_clear += [ | ||
| "sentry:seer_automation_handoff_point", | ||
| "sentry:seer_automation_handoff_target", | ||
| "sentry:seer_automation_handoff_integration_id", | ||
| ] | ||
| else: | ||
| integration_id = data.get("integrationId") | ||
| if integration_id is None: | ||
| raise ValueError("integrationId is required for external coding agents") | ||
| options_to_set["sentry:seer_automation_handoff_point"] = AutofixHandoffPoint.ROOT_CAUSE | ||
| options_to_set["sentry:seer_automation_handoff_target"] = agent | ||
| options_to_set["sentry:seer_automation_handoff_integration_id"] = integration_id | ||
|
|
||
| if "scannerAutomation" in data: | ||
| _set_or_clear("sentry:seer_scanner_automation", data["scannerAutomation"], default=True) | ||
|
|
||
| if "stoppingPoint" not in data: | ||
| return options_to_set, options_to_clear | ||
| elif data["stoppingPoint"] == "off": | ||
| # Disable automation and leave stopping point and handoff_auto_create_pr unchanged | ||
| # so that reenabling restores the prior state. | ||
| _set_or_clear( | ||
| "sentry:autofix_automation_tuning", | ||
| AutofixAutomationTuningSettings.OFF, | ||
| default=AUTOFIX_AUTOMATION_TUNING_DEFAULT, | ||
| ) | ||
| else: | ||
| # Enable automation and set the stopping point. | ||
| _set_or_clear( | ||
| "sentry:autofix_automation_tuning", | ||
| AutofixAutomationTuningSettings.MEDIUM, | ||
| default=AUTOFIX_AUTOMATION_TUNING_DEFAULT, | ||
| ) | ||
| _set_or_clear( | ||
| "sentry:seer_automated_run_stopping_point", | ||
| data["stoppingPoint"], | ||
| default=SEER_AUTOMATED_RUN_STOPPING_POINT_DEFAULT, | ||
| ) | ||
|
|
||
| if data["stoppingPoint"] == AutofixStoppingPoint.OPEN_PR: | ||
| # Safe to set even if no external handoff is configured | ||
| # since we'll only read it if the other handoff options are all non-null. | ||
| options_to_set["sentry:seer_automation_handoff_auto_create_pr"] = True | ||
| else: | ||
| options_to_clear.append("sentry:seer_automation_handoff_auto_create_pr") | ||
| return options_to_set, options_to_clear | ||
|
|
||
|
|
||
| def update_seer_project_settings(project: Project, data: SeerProjectSettingsUpdate) -> None: | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function's logic is unchanged, we just return the options to set and clear instead of doing it right there, so that we can do both single and bulk db ops. |
||
| """Apply high-level Seer settings to a project. Only update a setting if it's present in data.""" | ||
| """Apply high-level Seer settings to a single project.""" | ||
| options_to_set, options_to_delete = _get_seer_project_options_to_update(data) | ||
|
|
||
| def _set_if_not_default(key: str, value: Any, default: Any) -> None: | ||
| """If we're trying to set a default, delete the option. Otherwise, set it.""" | ||
| if value == default: | ||
| with transaction.atomic(using=router.db_for_write(ProjectOption)): | ||
| # Lock project rows to serialize concurrent writes. | ||
| Project.objects.select_for_update().filter(id=project.id).first() | ||
|
|
||
| for key in options_to_delete: | ||
|
srest2021 marked this conversation as resolved.
|
||
| project.delete_option(key) | ||
| else: | ||
| for key, value in options_to_set.items(): | ||
|
srest2021 marked this conversation as resolved.
|
||
| project.update_option(key, value) | ||
|
|
||
| with transaction.atomic(using=router.db_for_write(ProjectOption)): | ||
| list(Project.objects.select_for_update().filter(id=project.id)) | ||
|
|
||
| stopping_point: str | None = data.get("stoppingPoint") | ||
|
|
||
| if "agent" in data: | ||
| agent: str = data["agent"] | ||
| if agent == AutomationCodingAgent.SEER: | ||
| project.delete_option("sentry:seer_automation_handoff_point") | ||
| project.delete_option("sentry:seer_automation_handoff_target") | ||
| project.delete_option("sentry:seer_automation_handoff_integration_id") | ||
| else: | ||
| integration_id = data.get("integrationId") | ||
| if integration_id is None: | ||
| raise ValueError("integrationId is required for external coding agents") | ||
|
|
||
| project.update_option( | ||
| "sentry:seer_automation_handoff_point", AutofixHandoffPoint.ROOT_CAUSE | ||
| ) | ||
| project.update_option("sentry:seer_automation_handoff_target", agent) | ||
| project.update_option( | ||
| "sentry:seer_automation_handoff_integration_id", integration_id | ||
| ) | ||
| def bulk_update_seer_project_settings( | ||
| projects: list[Project], data: SeerProjectSettingsUpdate | ||
| ) -> None: | ||
| """Apply high-level Seer settings to multiple projects in bulk.""" | ||
| if not projects: | ||
| return | ||
|
|
||
| if stopping_point is not None: | ||
| if stopping_point == "off": | ||
| # Turn off tuning and leave stopping point and handoff_auto_create_pr unchanged | ||
| # so that reenabling restores the prior state. | ||
| _set_if_not_default( | ||
| "sentry:autofix_automation_tuning", | ||
| AutofixAutomationTuningSettings.OFF, | ||
| default=AUTOFIX_AUTOMATION_TUNING_DEFAULT, | ||
| ) | ||
| else: | ||
| _set_if_not_default( | ||
| "sentry:autofix_automation_tuning", | ||
| AutofixAutomationTuningSettings.MEDIUM, | ||
| default=AUTOFIX_AUTOMATION_TUNING_DEFAULT, | ||
| ) | ||
| _set_if_not_default( | ||
| "sentry:seer_automated_run_stopping_point", | ||
| stopping_point, | ||
| default=SEER_AUTOMATED_RUN_STOPPING_POINT_DEFAULT, | ||
| ) | ||
| options_to_set, options_to_delete = _get_seer_project_options_to_update(data) | ||
| if not options_to_set and not options_to_delete: | ||
| return | ||
|
|
||
| if stopping_point == AutofixStoppingPoint.OPEN_PR: | ||
| # Safe to set even if no external handoff is configured | ||
| # since we'll only read it if the other handoff options are all non-null. | ||
| project.update_option("sentry:seer_automation_handoff_auto_create_pr", True) | ||
| else: | ||
| project.delete_option("sentry:seer_automation_handoff_auto_create_pr") | ||
| project_ids = [p.id for p in projects] | ||
|
|
||
| if "scannerAutomation" in data: | ||
| _set_if_not_default( | ||
| "sentry:seer_scanner_automation", data["scannerAutomation"], default=True | ||
| with transaction.atomic(using=router.db_for_write(ProjectOption)): | ||
| # Lock project rows to serialize concurrent writes. | ||
| list(Project.objects.select_for_update().filter(id__in=project_ids).order_by("id")) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Necessary to select for update, because we are modifying multiple related options that should be treated as a group (like handoff). Here is our current project count distribution--99.9% of our orgs have less than 500 projects, 99.34% less than 25. Given those stats, wondering if it's necessary to batch so that we don't lock up many projects at once / cause timeouts? I don't see any precedent for this in other code, but figured I'd bring it up |
||
|
|
||
| if options_to_delete: | ||
| # Use _raw_delete to skip per-row post_delete signals that each trigger reload_cache. | ||
| # For efficiency, we reload once per project after the transaction instead. | ||
| ProjectOption.objects.filter( | ||
| project_id__in=project_ids, key__in=options_to_delete | ||
| )._raw_delete(using=router.db_for_write(ProjectOption)) | ||
|
|
||
| if options_to_set: | ||
| ProjectOption.objects.bulk_create( | ||
| [ | ||
| ProjectOption(project_id=pid, key=key, value=value) | ||
| for pid in project_ids | ||
| for key, value in options_to_set.items() | ||
| ], | ||
| update_conflicts=True, | ||
| unique_fields=["project_id", "key"], | ||
| update_fields=["value"], | ||
| ) | ||
|
|
||
| # Manually reload each project's cache, since _raw_delete and bulk_create | ||
| # bypass the cache reloading in update_option and delete_option. | ||
| for project_id in project_ids: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if this ends up being prohibitively slow, we could always move this to an async task. |
||
| ProjectOption.objects.reload_cache(project_id, "projectoption.bulk_set_value") | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that we always reload the cache for each project, instead of checking first if any options were actually changed like |
||
|
|
||
|
|
||
| def has_project_connected_repos(organization: Organization, project: Project) -> bool: | ||
| """Check if a project has connected repositories for Seer automation.""" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ | |
| CodingAgentProviderType, | ||
| CodingAgentStatus, | ||
| bulk_read_preferences_from_sentry_db, | ||
| bulk_update_seer_project_settings, | ||
| bulk_write_preferences_to_sentry_db, | ||
| clear_preference_automation_handoff, | ||
| deduplicate_repositories, | ||
|
|
@@ -1762,3 +1763,166 @@ def test_deletes_option_when_value_is_default(self) -> None: | |
| assert not ProjectOption.objects.filter( | ||
| project=self.project, key="sentry:seer_scanner_automation" | ||
| ).exists() | ||
|
|
||
|
|
||
| class TestBulkUpdateSeerProjectSettings(TestCase): | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kept this pretty simple since the business logic is already tested in |
||
| def setUp(self) -> None: | ||
| super().setUp() | ||
| self.project_a = self.create_project(organization=self.organization) | ||
| self.project_b = self.create_project(organization=self.organization) | ||
| self.projects = [self.project_a, self.project_b] | ||
|
|
||
| def test_empty_projects(self) -> None: | ||
| """Empty project list should be a no-op without errors.""" | ||
| bulk_update_seer_project_settings([], {"scannerAutomation": False}) | ||
|
|
||
| def test_sets_options(self) -> None: | ||
| """All provided settings fields should be applied to every project.""" | ||
| bulk_update_seer_project_settings( | ||
| self.projects, | ||
| { | ||
| "agent": AutomationCodingAgent.CURSOR, | ||
| "integrationId": 99, | ||
| "stoppingPoint": AutofixStoppingPoint.OPEN_PR, | ||
| "scannerAutomation": False, | ||
| }, | ||
| ) | ||
|
|
||
| for project in self.projects: | ||
| assert ( | ||
| project.get_option("sentry:seer_automation_handoff_target") | ||
| == AutomationCodingAgent.CURSOR | ||
| ) | ||
| assert ( | ||
| project.get_option("sentry:seer_automation_handoff_point") | ||
| == AutofixHandoffPoint.ROOT_CAUSE | ||
| ) | ||
| assert project.get_option("sentry:seer_automation_handoff_integration_id") == 99 | ||
| assert ( | ||
| project.get_option("sentry:autofix_automation_tuning") | ||
| == AutofixAutomationTuningSettings.MEDIUM | ||
| ) | ||
| assert ( | ||
| project.get_option("sentry:seer_automated_run_stopping_point") | ||
| == AutofixStoppingPoint.OPEN_PR | ||
| ) | ||
| assert project.get_option("sentry:seer_automation_handoff_auto_create_pr") is True | ||
| assert project.get_option("sentry:seer_scanner_automation") is False | ||
|
|
||
| def test_agent_seer_clears_handoff_options(self) -> None: | ||
| """Switching to seer agent should delete handoff options across all projects.""" | ||
| for project in self.projects: | ||
| project.update_option( | ||
| "sentry:seer_automation_handoff_target", | ||
| CodingAgentProviderType.CURSOR_BACKGROUND_AGENT, | ||
| ) | ||
| project.update_option( | ||
| "sentry:seer_automation_handoff_point", AutofixHandoffPoint.ROOT_CAUSE | ||
| ) | ||
| project.update_option("sentry:seer_automation_handoff_integration_id", 42) | ||
|
|
||
| bulk_update_seer_project_settings(self.projects, {"agent": AutomationCodingAgent.SEER}) | ||
|
|
||
| for project in self.projects: | ||
| assert project.get_option("sentry:seer_automation_handoff_target") is None | ||
| assert project.get_option("sentry:seer_automation_handoff_point") is None | ||
| assert project.get_option("sentry:seer_automation_handoff_integration_id") is None | ||
|
|
||
| def test_upserts_existing_options(self) -> None: | ||
| """Existing options should be overwritten, not duplicated.""" | ||
| for project in self.projects: | ||
| project.update_option("sentry:seer_scanner_automation", True) | ||
|
|
||
| bulk_update_seer_project_settings(self.projects, {"scannerAutomation": False}) | ||
|
|
||
| for project in self.projects: | ||
| assert project.get_option("sentry:seer_scanner_automation") is False | ||
| assert ( | ||
| ProjectOption.objects.filter( | ||
| project=project, key="sentry:seer_scanner_automation" | ||
| ).count() | ||
| == 1 | ||
| ) | ||
|
|
||
| def test_clears_option_when_value_is_default(self) -> None: | ||
| """Setting a value equal to its registered default should delete the ProjectOption row.""" | ||
| for project in self.projects: | ||
| project.update_option("sentry:seer_automated_run_stopping_point", "open_pr") | ||
|
|
||
| bulk_update_seer_project_settings( | ||
| self.projects, | ||
| {"stoppingPoint": AutofixStoppingPoint(SEER_AUTOMATED_RUN_STOPPING_POINT_DEFAULT)}, | ||
| ) | ||
|
|
||
| for project in self.projects: | ||
| assert ( | ||
| project.get_option("sentry:seer_automated_run_stopping_point") | ||
| == SEER_AUTOMATED_RUN_STOPPING_POINT_DEFAULT | ||
| ) | ||
| assert not ProjectOption.objects.filter( | ||
| project=project, key="sentry:seer_automated_run_stopping_point" | ||
| ).exists() | ||
|
|
||
| def test_stopping_point_off_sets_tuning_off(self) -> None: | ||
| """stoppingPoint='off' should set tuning to OFF and preserve existing stopping point.""" | ||
| for project in self.projects: | ||
| project.update_option( | ||
| "sentry:seer_automated_run_stopping_point", AutofixStoppingPoint.OPEN_PR | ||
| ) | ||
| project.update_option("sentry:seer_automation_handoff_auto_create_pr", True) | ||
|
|
||
| bulk_update_seer_project_settings(self.projects, {"stoppingPoint": "off"}) | ||
|
|
||
| for project in self.projects: | ||
| assert ( | ||
| project.get_option("sentry:autofix_automation_tuning") | ||
| == AutofixAutomationTuningSettings.OFF | ||
| ) | ||
|
|
||
| def test_mixed_sets_and_clears_options(self) -> None: | ||
| """Test that sets new options and deletes existing ones.""" | ||
| for project in self.projects: | ||
| project.update_option( | ||
| "sentry:seer_automation_handoff_target", | ||
| CodingAgentProviderType.CURSOR_BACKGROUND_AGENT, | ||
| ) | ||
| project.update_option( | ||
| "sentry:seer_automation_handoff_point", AutofixHandoffPoint.ROOT_CAUSE | ||
| ) | ||
| project.update_option("sentry:seer_automation_handoff_integration_id", 42) | ||
|
|
||
| bulk_update_seer_project_settings( | ||
| self.projects, | ||
| {"agent": AutomationCodingAgent.SEER, "scannerAutomation": False}, | ||
| ) | ||
|
|
||
| for project in self.projects: | ||
| assert project.get_option("sentry:seer_automation_handoff_target") is None | ||
| assert project.get_option("sentry:seer_automation_handoff_point") is None | ||
| assert project.get_option("sentry:seer_automation_handoff_integration_id") is None | ||
| assert project.get_option("sentry:seer_scanner_automation") is False | ||
|
|
||
| def test_omitted_fields_preserve_existing_options(self) -> None: | ||
| """Updating one field should not clobber unrelated existing options.""" | ||
| for project in self.projects: | ||
| project.update_option( | ||
| "sentry:autofix_automation_tuning", AutofixAutomationTuningSettings.MEDIUM | ||
| ) | ||
| project.update_option( | ||
| "sentry:seer_automated_run_stopping_point", AutofixStoppingPoint.OPEN_PR | ||
| ) | ||
| project.update_option("sentry:seer_automation_handoff_auto_create_pr", True) | ||
|
|
||
| bulk_update_seer_project_settings(self.projects, {"scannerAutomation": False}) | ||
|
|
||
| for project in self.projects: | ||
| assert ( | ||
| project.get_option("sentry:autofix_automation_tuning") | ||
| == AutofixAutomationTuningSettings.MEDIUM | ||
| ) | ||
| assert ( | ||
| project.get_option("sentry:seer_automated_run_stopping_point") | ||
| == AutofixStoppingPoint.OPEN_PR | ||
| ) | ||
| assert project.get_option("sentry:seer_automation_handoff_auto_create_pr") is True | ||
| assert project.get_option("sentry:seer_scanner_automation") is False | ||
Uh oh!
There was an error while loading. Please reload this page.