ref(cells): Region -> cell migration in integrations modules#111130
ref(cells): Region -> cell migration in integrations modules#111130
Conversation
github-actions
bot
added
the
Scope: Backend
| # TODO(cells): This is broken for a multi-cell locality as the router cannot identify | ||
| # the correct cell silo for routing. The endpoint should be moved to the control silo. | ||
| url_prefix=( | ||
| generate_locality_url() if SiloMode.get_current_mode() == SiloMode.CELL else None |
There was a problem hiding this comment.
What if we just pass None here in all cases? Does that force the view through apigateway and to the correct cell?
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| raise NotImplementedError("Integration requires an identity") | ||
| default_auth_id = identity_model.id | ||
| if self.provider.is_region_restricted and is_violating_cell_restriction( | ||
| if self.provider.is_cell_restricted and is_violating_cell_restriction( |
There was a problem hiding this comment.
Bug: Tests in test_pipeline.py were not updated to use the new is_cell_restricted attribute, instead referencing the old is_region_restricted, breaking test coverage for this feature.
Severity: HIGH
Suggested Fix
Update the tests in tests/sentry/integrations/test_pipeline.py to use the new attribute name is_cell_restricted instead of is_region_restricted. This applies to patch.multiple calls and direct attribute assignments like self.provider.is_cell_restricted = True.
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/pipeline.py#L301
Potential issue: The production code correctly renamed the `is_region_restricted`
attribute to `is_cell_restricted` on the `IntegrationProvider` base class and updated
the integration pipeline to check this new attribute. However, tests in
`tests/sentry/integrations/test_pipeline.py` were not updated. They continue to patch
and set the old `is_region_restricted` attribute. Because the production code now checks
`self.provider.is_cell_restricted`, which remains `False` during tests, the cell
restriction logic is never actually executed. This breaks the tests and creates a gap in
test coverage for the cell restriction feature, meaning bugs in this logic may not be
caught.
Did we get this right? 👍 / 👎 to inform future reviews.
2 participants
No description provided.