feat(cells): Support multi-cell jira integration#111696
Conversation
note: this is behind a feature flag for safety, the default is unchanged (no multi-cell) and merging the code should be a no-op
| @property | ||
| def is_cell_restricted(self) -> bool: | ||
| """ | ||
| Returns True if each integration installation can only be connected on one cell of Sentry at a | ||
| time. It will raise an error if any organization from another cell attempts to install it. | ||
| """ | ||
| return False |
There was a problem hiding this comment.
Bug: Changing is_cell_restricted to a read-only @property will cause an AttributeError in tests that attempt to directly assign a value to it, leading to test failures.
Severity: HIGH
Suggested Fix
Update the tests that assign to self.provider.is_cell_restricted. Instead of direct assignment, use a mocking technique to set the property's return value for the test's duration. For example, use mock.patch.object with new_callable=mock.PropertyMock to temporarily override the property's behavior without triggering an AttributeError.
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/base.py#L245-L251
Potential issue: The `is_cell_restricted` attribute in the `IntegrationProvider` class
was converted into a read-only `@property`. However, multiple tests in
`tests/sentry/integrations/test_pipeline.py` still attempt to directly assign a value to
this property, such as `self.provider.is_cell_restricted = True`. Since a read-only
property does not have a setter, these assignment attempts will raise an
`AttributeError` at runtime. This will cause any test that calls the
`_setup_cell_restriction()` helper method or directly sets the property to fail,
blocking the test suite from passing.
Did we get this right? 👍 / 👎 to inform future reviews.
| "target": { | ||
| "type": "web_panel", | ||
| "url": "/extensions/jira/issue/{issue.key}/", | ||
| "url": "/extensions/jira/issue-details/{issue.key}/", |
There was a problem hiding this comment.
I don't know if Jira snapshots this into the Jira instances when the extension is added. We might need to keep the old URL around as well.
| sentry_sdk.capture_exception(err) | ||
| return self.get_response_from_control_silo() | ||
| # TODO(cells): Remove once all installs have migrated to JiraSentryIssueDetailsControlView. | ||
| return JiraSentryIssueDetailsControlView.as_view()(self.request, **self.match.kwargs) |
There was a problem hiding this comment.
This looks reasonable to me. This endpoint was added in #98324 with the intention of replacing the region scoped endpoint, but the transition work wasn't completed.
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.
Backend Test FailuresFailures on
|
Backend Test FailuresFailures on
|

note: this is behind a feature flag for safety, the default is unchanged (no multi-cell) and merging the code should be a no-op