KEMBAR78
ref: fix some types for sentry.issues by asottile-sentry · Pull Request #72813 · getsentry/sentry · GitHub
Skip to content

Conversation

@asottile-sentry
Copy link
Contributor

No description provided.

@asottile-sentry asottile-sentry requested review from a team June 14, 2024 20:49
@asottile-sentry asottile-sentry requested review from a team as code owners June 14, 2024 20:49
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 14, 2024
)

union_qs = Activity.objects.none()
union_qs = Activity.objects.none().values_list("id", flat=True)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this convinces mypy the type is QuerySet[Activity, int]

sort=result["sort"],
visibility=result["visibility"],
)
assert saved_search.type is not None
Copy link
Member

@joshuarli joshuarli Jun 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't it be better to check result["type"] in the validator? so we can return a proper http 400

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is checked there -- but the model can do some defaulting on type so I have to check it here as well

technically the database schema is wrong (it should be NOT NULL for that column) but fixing that is a lot more involved

@asottile-sentry asottile-sentry merged commit fcb6eef into master Jun 17, 2024
@asottile-sentry asottile-sentry deleted the asottile-typing-issues branch June 17, 2024 13:36
@github-actions github-actions bot locked and limited conversation to collaborators Jul 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants