KEMBAR78
ref: culprit is nullable by asottile-sentry · Pull Request #75154 · getsentry/sentry · GitHub
Skip to content

Conversation

@asottile-sentry
Copy link
Contributor

when mypy checks models it notices that construction passes the culprit (nullable) column to this

@asottile-sentry asottile-sentry requested review from a team July 29, 2024 14:25
@asottile-sentry asottile-sentry requested a review from a team as a code owner July 29, 2024 14:25
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 29, 2024
@codecov
Copy link

codecov bot commented Jul 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.10%. Comparing base (477d4ec) to head (73b383d).
Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #75154      +/-   ##
==========================================
- Coverage   78.10%   78.10%   -0.01%     
==========================================
  Files        6757     6757              
  Lines      301554   301554              
  Branches    51888    51888              
==========================================
- Hits       235525   235524       -1     
- Misses      59686    59689       +3     
+ Partials     6343     6341       -2     
Files Coverage Δ
.../sentry/api/endpoints/organization_events_trace.py 85.56% <100.00%> (ø)

... and 11 files with indirect coverage changes

@asottile-sentry asottile-sentry merged commit 56154a1 into master Jul 29, 2024
@asottile-sentry asottile-sentry deleted the asottile-culprit-nullable branch July 29, 2024 17:14
asottile-sentry added a commit that referenced this pull request Jul 29, 2024
an absolute ton of work went into enabling this -- but after this PR
model types / create / update / etc. should mostly be typechecked
whereas they were entirely unchecked (everything `Any`) prior to this.

most of the core problem was that mypy and django-stubs did not
understand our custom `BaseManager` and `BaseQuerySet` since there's a
significant amount of django stuff that goes into making those classes
"real"

fix that involved these issues:

- python/mypy#17402 (worked around)
- typeddjango/django-stubs#2228 (the workaround)

with that fixed, it exposed a handful issues in django-stubs itself:

- typeddjango/django-stubs#2240
- typeddjango/django-stubs#2241
- typeddjango/django-stubs#2244
- typeddjango/django-stubs#2248
- typeddjango/django-stubs#2276
- typeddjango/django-stubs#2281

fixing all of those together exposed around 1k issues in sentry code
itself which was fixed over a number of PRs:
- #75186
- #75108
- #75149
- #75162
- #75150
- #75154
- #75158
- #75148
- #75157
- #75156
- #75152
- #75153
- #75151
- #75113
- #75112
- #75111
- #75110
- #75109
- #75013
- #74641
- #74640
- #73783
- #73787
- #73788
- #73793
- #73791
- #73786
- #73761
- #73742
- #73744
- #73602
- #73596
- #73595
- #72892
- #73589
- #73587
- #73581
- #73580
- #73213
- #73207
- #73206
- #73205
- #73202
- #73198
- #73121
- #73109
- #73073
- #73061
- getsentry/getsentry#14370
- #72965
- #72963
- #72967
- #72877 (eventually was able to remove this when upgrading to mypy
1.11)
- #72948
- #72799
- #72898
- #72899
- #72896
- #72895
- #72903
- #72894
- #72897
- #72893
- #72889
- #72887
- #72888
- #72811
- #72872
- #72813
- #72815
- #72812
- #72808
- #72802
- #72807
- #72809
- #72810
- #72814
- #72816
- #72818
- #72806
- #72801
- #72797
- #72800
- #72798
- #72771
- #72718
- #72719
- #72710
- #72709
- #72706
- #72693
- #72641
- #72591
- #72635


mypy 1.11 also included some important improvements with typechecking
`for model_cls in (M1, M2, M2): ...` which also exposed some problems in
our code. unfortunately upgrading mypy involved fixing a lot of things
as well:
- #75020
- #74982
- #74976
- #74974
- #74972
- #74967
- #74954
- getsentry/getsentry#14739
- getsentry/getsentry#14734
- #74959
- #74958
- #74956
- #74953
- #74955
- #74952
- #74895
- #74892
- #74897
- #74896
- #74893
- #74880
- #74900
- #74902
- #74903
- #74904
- #74899
- #74894
- #74882
- #74881
- #74871
- #74870
- #74855
- #74856
- #74853
- #74857
- #74858
- #74807
- #74805
- #74803
- #74806
- #74798
- #74809
- #74801
- #74804
- #74800
- #74799
- #74725
- #74682
- #74677
- #74680
- #74683
- #74681


needless to say this is probably the largest stacked PR I've ever done
-- and I'm pretty happy with how I was able to split this up into
digestable chunks

<!-- Describe your PR here. -->
roaga pushed a commit that referenced this pull request Jul 31, 2024
an absolute ton of work went into enabling this -- but after this PR
model types / create / update / etc. should mostly be typechecked
whereas they were entirely unchecked (everything `Any`) prior to this.

most of the core problem was that mypy and django-stubs did not
understand our custom `BaseManager` and `BaseQuerySet` since there's a
significant amount of django stuff that goes into making those classes
"real"

fix that involved these issues:

- python/mypy#17402 (worked around)
- typeddjango/django-stubs#2228 (the workaround)

with that fixed, it exposed a handful issues in django-stubs itself:

- typeddjango/django-stubs#2240
- typeddjango/django-stubs#2241
- typeddjango/django-stubs#2244
- typeddjango/django-stubs#2248
- typeddjango/django-stubs#2276
- typeddjango/django-stubs#2281

fixing all of those together exposed around 1k issues in sentry code
itself which was fixed over a number of PRs:
- #75186
- #75108
- #75149
- #75162
- #75150
- #75154
- #75158
- #75148
- #75157
- #75156
- #75152
- #75153
- #75151
- #75113
- #75112
- #75111
- #75110
- #75109
- #75013
- #74641
- #74640
- #73783
- #73787
- #73788
- #73793
- #73791
- #73786
- #73761
- #73742
- #73744
- #73602
- #73596
- #73595
- #72892
- #73589
- #73587
- #73581
- #73580
- #73213
- #73207
- #73206
- #73205
- #73202
- #73198
- #73121
- #73109
- #73073
- #73061
- getsentry/getsentry#14370
- #72965
- #72963
- #72967
- #72877 (eventually was able to remove this when upgrading to mypy
1.11)
- #72948
- #72799
- #72898
- #72899
- #72896
- #72895
- #72903
- #72894
- #72897
- #72893
- #72889
- #72887
- #72888
- #72811
- #72872
- #72813
- #72815
- #72812
- #72808
- #72802
- #72807
- #72809
- #72810
- #72814
- #72816
- #72818
- #72806
- #72801
- #72797
- #72800
- #72798
- #72771
- #72718
- #72719
- #72710
- #72709
- #72706
- #72693
- #72641
- #72591
- #72635


mypy 1.11 also included some important improvements with typechecking
`for model_cls in (M1, M2, M2): ...` which also exposed some problems in
our code. unfortunately upgrading mypy involved fixing a lot of things
as well:
- #75020
- #74982
- #74976
- #74974
- #74972
- #74967
- #74954
- getsentry/getsentry#14739
- getsentry/getsentry#14734
- #74959
- #74958
- #74956
- #74953
- #74955
- #74952
- #74895
- #74892
- #74897
- #74896
- #74893
- #74880
- #74900
- #74902
- #74903
- #74904
- #74899
- #74894
- #74882
- #74881
- #74871
- #74870
- #74855
- #74856
- #74853
- #74857
- #74858
- #74807
- #74805
- #74803
- #74806
- #74798
- #74809
- #74801
- #74804
- #74800
- #74799
- #74725
- #74682
- #74677
- #74680
- #74683
- #74681


needless to say this is probably the largest stacked PR I've ever done
-- and I'm pretty happy with how I was able to split this up into
digestable chunks

<!-- Describe your PR here. -->
@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 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