ref(integrations): Remove the legacy web-view setup pipeline from integrations#117362
Conversation
|
|
||
| setup_dialog_config = {"width": 600, "height": 900} | ||
|
|
||
| def get_pipeline_views(self) -> list[PipelineView[IntegrationPipeline]]: | ||
| return [] | ||
|
|
||
| def get_pipeline_api_steps(self) -> ApiPipelineSteps[IntegrationPipeline]: | ||
| return [PagerDutyInstallationApiStep()] | ||
|
|
There was a problem hiding this comment.
Bug: Removing get_pipeline_views() from integration provider subclasses without removing the @abc.abstractmethod from the parent class will cause a TypeError on instantiation.
Severity: CRITICAL
Suggested Fix
Remove the @abc.abstractmethod decorator from get_pipeline_views() in the PipelineProvider base class. Alternatively, provide a default implementation (e.g., return []) in the IntegrationProvider class to satisfy the abstract contract for all its subclasses.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/integrations/pagerduty/integration.py#L243-L248
Potential issue: The `PipelineProvider` class defines `get_pipeline_views()` as an
abstract method. The pull request removes the concrete implementation of this method
from all `IntegrationProvider` subclasses (e.g., `PagerDutyIntegrationProvider`).
Because the parent `IntegrationProvider` does not implement this method, all its
subclasses become abstract. The `IntegrationManager.get()` method attempts to
instantiate these subclasses, which will raise a `TypeError` because an abstract class
cannot be instantiated. This will cause all integration setup and pipeline
initializations to fail at runtime.
Also affects:
src/sentry/integrations/aws_lambda/integration.py:407~412src/sentry/integrations/bitbucket/integration.py:260~265src/sentry/integrations/bitbucket_server/integration.py:381~386src/sentry/integrations/claude_code/integration.py:185~190src/sentry/integrations/cursor/integration.py:90~95src/sentry/integrations/discord/integration.py:257~262src/sentry/integrations/example/integration.py:240~245src/sentry/integrations/github/integration.py:742~747src/sentry/integrations/github_copilot/integration.py:61~66src/sentry/integrations/github_enterprise/integration.py:664~669src/sentry/integrations/gitlab/integration.py:666~671src/sentry/integrations/jira/integration.py:1309~1314src/sentry/integrations/jira_server/integration.py:1398~1403src/sentry/integrations/msteams/integration.py:188~193src/sentry/integrations/opsgenie/integration.py:238~243src/sentry/integrations/perforce/integration.py:651~656src/sentry/integrations/slack/integration.py:367~372src/sentry/integrations/vercel/integration.py:479~484src/sentry/integrations/vsts/integration.py:605~610
Did we get this right? 👍 / 👎 to inform future reviews.
b3b3873 to
b14078a
Compare
b14078a to
20a6931
Compare
… type (#117370) The integration setup popup that consumed `provider.setupDialog` (window URL and dimensions) was removed when integrations moved to the API-driven pipeline modal, and the backend no longer serializes the field. This drops the now-unused `setupDialog` from the `IntegrationProvider` type, along with the fixtures, stories, and spec that set it. Backend removal of the field is in #117362. Part of [VDY-32](https://linear.app/getsentry/issue/VDY-32/migrate-integration-setup-pipelines-to-api-driven-flows).
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 20a6931. Configure here.
| integration_cls: type[IntegrationInstallation] | None = None | ||
| """an Integration class that will manage the functionality once installed""" | ||
|
|
||
| setup_dialog_config = {"width": 600, "height": 600} |
There was a problem hiding this comment.
Orphan docstring after config removal
Low Severity
Deleting setup_dialog_config left a standalone """configuration for the setup dialog""" string on IntegrationProvider with no associated attribute, which is misleading dead code left from an incomplete removal.
Reviewed by Cursor Bugbot for commit 20a6931. Configure here.
…egrations Every integration provider now installs through the API-driven pipeline (`get_pipeline_api_steps()` + the organization pipeline endpoint), so the server-rendered web-view setup path is dead for integrations. Drop `get_pipeline_views()` from the `IntegrationProvider` base class and from all provider subclasses (each was a no-op `return []`). `IntegrationPipeline` now returns an empty list of pipeline views directly, so integration pipelines no longer depend on the provider method. Delete the now-unused `OrganizationIntegrationSetupView` and its `/extensions/<provider>/setup/` route, plus the dangling test helper. The generic `Pipeline` framework (and `PipelineView`/`PipelineAdvancerView`) is left intact since it is still shared with the auth/SSO and identity pipelines.
20a6931 to
47f1807
Compare


Every integration provider now installs through the API-driven pipeline (
get_pipeline_api_steps()+ theorganization_pipelineendpoint + the frontend modal), so the server-rendered web-view setup path is dead for integrations.This removes it from the integration layer:
get_pipeline_views()from theIntegrationProviderbase class and from all provider subclasses (each was a no-opreturn []).IntegrationPipelinenow returns an empty list of pipeline views directly, so integration pipelines no longer depend on the provider method.OrganizationIntegrationSetupViewand its/organizations/.../integrations/<provider>/setup/route, plus a dangling test helper.The generic
Pipelineframework is intentionally left intact —PipelineView,NestedPipelineView, andPipelineAdvancerView(thesentry-extension-setupOAuth-callback trampoline that the API flow still relies on) are shared with the auth/SSO and identity pipelines.Part of VDY-32.