Conversation
…tion and platform name tweaks
| "platform_name": ["platform_name"], | ||
| "git_head_ref": ["git_head_ref"], | ||
| "app_id": ["app_id"], | ||
| "build_configuration": ["build_configuration"], | ||
| "build_configuration_name": ["build_configuration_name"], | ||
| }, | ||
| allowed_keys={ | ||
| "platform", | ||
| "platform_name", | ||
| "git_head_ref", | ||
| "app_id", | ||
| "build_configuration", | ||
| "build_configuration_name", | ||
| }, |
There was a problem hiding this comment.
Bug: The updated preprod_artifact_search_config expects new filter keys (platform_name), but existing saved rules use old keys (platform), causing them to fail parsing and never match.
Severity: HIGH
Suggested Fix
A data migration should be added to update all existing saved status check filter queries in project options. The migration should replace the old keys (platform, build_configuration) with the new keys (platform_name, build_configuration_name). Alternatively, the parsing logic could be updated to support both old and new keys for backward compatibility.
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/preprod/vcs/status_checks/size/tasks.py#L55-L71
Potential issue: The change to `preprod_artifact_search_config` renames filter keys from
`platform` and `build_configuration` to `platform_name` and `build_configuration_name`.
However, existing status check rules stored in the database have filter queries using
the old keys, such as `platform:ios`. When `_rule_matches_artifact` attempts to parse
these old queries with the new config, it will raise an `InvalidSearchQuery` exception.
This exception is caught, and the function returns `False`, causing all existing status
check rules with filters to silently fail and never trigger, breaking existing user
configurations.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
this feature hasn't been released yet, so no need to worry about backwards compatibility
Required backend update to land if #106806 merges