KEMBAR78
ref: fill out TypeVars for ForeignKey by asottile-sentry · Pull Request #75228 · getsentry/sentry · GitHub
Skip to content

Conversation

@asottile-sentry
Copy link
Contributor

@asottile-sentry asottile-sentry commented Jul 30, 2024

this fixes a class of problem when upgrading to django-stubs 5.0.4 where _ST becomes unbound

in order to fix this I first had to adjust the TypeVars in django-stubs itself:

fixing that mypy now followed foreign keys which resulted in a few new errors (~30) needing fixing in sentry:

fixing those left about ~5 errors that mypy was flagging code as "unreachable" but the foreign keys were marked null=True -- this turned out to be another bug in django-stubs related to how our "FlexibleForeignKey" was defined:

fixing that surfaced ~220 new errors in sentry which were fixed:

@asottile-sentry asottile-sentry requested review from a team July 30, 2024 13:38
@asottile-sentry asottile-sentry requested a review from a team as a code owner July 30, 2024 13:38
@asottile-sentry asottile-sentry enabled auto-merge (squash) July 30, 2024 13:38
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 30, 2024
@codecov
Copy link

codecov bot commented Jul 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.16%. Comparing base (a796dbe) to head (507a913).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #75228      +/-   ##
==========================================
+ Coverage   78.02%   78.16%   +0.13%     
==========================================
  Files        6765     6765              
  Lines      301819   301820       +1     
  Branches    51922    51922              
==========================================
+ Hits       235506   235909     +403     
+ Misses      59941    59557     -384     
+ Partials     6372     6354      -18     
Files Coverage Δ
src/sentry/db/models/fields/foreignkey.py 100.00% <100.00%> (ø)

... and 65 files with indirect coverage changes

this fixes a class of problem when upgrading to django-stubs 4.0.3
@asottile-sentry asottile-sentry merged commit 679ee11 into master Jul 31, 2024
@asottile-sentry asottile-sentry deleted the asottile-typevars-foreignkey branch July 31, 2024 17:08
roaga pushed a commit that referenced this pull request Jul 31, 2024
this fixes a class of problem when upgrading to django-stubs 4.0.3 where
`_ST` becomes unbound

<!-- Describe your PR here. -->
asottile-sentry added a commit that referenced this pull request Jul 31, 2024
<!-- Describe your PR here. -->

this had three sets of breakage addressed by other PRs:

- our foreign key subclass was not functioning, django-stubs added a
default TypeVar here which started getting filled in with an unbound
TypeVar resulting in thousands of errors: fixed by
#75228
- we were able to remove our fork's [descriptor
patch](getsentry/sentry-forked-django-stubs#4)
(which removed the non-model overload of `__get__` for fields) as mypy
[fixed this issue](python/mypy#17381). in doing
so it pointed out an unsafe descriptor access through a mixin and so
that had to go: #75360
- django-stubs improved some field validation through QuerySets which
was only checked through managers before: fixed by
#75359
@github-actions github-actions bot locked and limited conversation to collaborators Aug 16, 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.

2 participants