ref(cells): Update rpc service methods#110897
Conversation
github-actions
Bot
added
the
Scope: Backend
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| def update_action_status_for_sentry_app_installation( | ||
| self, | ||
| *, | ||
| region_name: str, | ||
| cell_name: str | None = None, # TODO(cells): make required when all callers are updated | ||
| region_name: str | None = None, # TODO(cells): remove when all callers are updated | ||
| status: int, | ||
| organization_id: int, | ||
| sentry_app_id: int, |
There was a problem hiding this comment.
Bug: The parameters in the concrete service implementations do not match the updated abstract service interfaces, which will cause a startup-time crash.
Severity: CRITICAL
Suggested Fix
Update the method signatures in the concrete service implementations (e.g., DatabaseBackedActionService) to match the abstract interfaces in service.py. This includes adding the cell_name parameter to methods like update_action_status_for_sentry_app_installation.
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/workflow_engine/service/action/service.py#L39-L46
Potential issue: The RPC service interfaces in `ActionService` and `IssueService` were
updated to add a new `cell_name` parameter and make `region_name` optional. However, the
corresponding concrete implementations in `impl.py` files were not updated to match
these new signatures. This discrepancy will cause the application to fail at startup.
The service initialization logic in `_get_and_validate_local_implementation` compares
the parameter sets of the abstract service and the concrete implementation. Since the
sets do not match (the implementation is missing `cell_name`), an
`RpcServiceSetupException` will be raised, preventing the application from starting.
Did we get this right? 👍 / 👎 to inform future reviews.
lynnagara
changed the title
lynnagara
changed the title
![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.
| self, *, region_name: str, external_issue_key: str, integration_id: int | ||
| self, | ||
| *, | ||
| cell_name: str | None = None, # TODO(cells): make required when all callers are updated |
There was a problem hiding this comment.
Should cell_name be at the end of the definition to begin with so that it doesn't throw off any *args calls, if any?
There was a problem hiding this comment.
the * is there (similar to the other rpc functions) so can only be called with keyword args and shouldn't have ordering issues when called
There was a problem hiding this comment.
Ahh, I forgot that it enforces it, lgtm
updates rpc methods to accept cell_name in: - workflow service - issue service - hook service - sentry_apps service - projects service - test generation service
2 participants
updates rpc methods to accept cell_name in: