ref: get proper base manager typing#72221
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #72221 +/- ##
==========================================
- Coverage 78.09% 78.08% -0.01%
==========================================
Files 6758 6758
Lines 301731 301732 +1
Branches 51925 51925
==========================================
- Hits 235624 235617 -7
- Misses 59761 59765 +4
- Partials 6346 6350 +4
|
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
171cebb to
586ecf4
Compare
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
586ecf4 to
424f31b
Compare
424f31b to
0d84170
Compare
| def _get_chunked_blob(self, mode=None, prefetch=False, prefetch_to=None, delete=True): | ||
| return ChunkedFileBlobIndexWrapper( | ||
| self.FILE_BLOB_INDEX_MODEL.objects.filter(file=self) | ||
| # TODO: file blob inheritance hierarchy is unsound |
There was a problem hiding this comment.
talked a bit with @markstory about these particular models -- we believe there's a way to convert these to free functions to capture the dependent-types inheritance hierarchy with the type system and make these sound -- but as a followup
| def update_membership_flags(self, *, organization_member: RpcOrganizationMember) -> None: | ||
| model = OrganizationMember.objects.get(id=organization_member.id) | ||
| model.flags = self._deserialize_member_flags(organization_member.flags) | ||
| model.flags = self._deserialize_member_flags(organization_member.flags) # type: ignore[assignment] # TODO: make BitField a mypy plugin |
There was a problem hiding this comment.
the issue here is HC converts the bitfield to an integer over the wire and the assignment then confuses our workaround for bitfields
I plan to follow up by making bitfield an actual mypy plugin which understands the assignments and values properly here
|
note that there are a number of "newly identified errors" in ignored files (~50-100) -- I'm deferring fixing those for now and will fix those as they get opted into type checking |
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
BaseManagerandBaseQuerySetsince there's a significant amount of django stuff that goes into making those classes "real"fix that involved these issues:
get_dynamic_class_hookdoes not handledeferproperly python/mypy#17402 (worked around)with that fixed, it exposed a handful issues in django-stubs itself:
objectsannotation typeddjango/django-stubs#2241fixing all of those together exposed around 1k issues in sentry code itself which was fixed over a number of PRs:
Nonekeys #75156project=passed toProjectTemplate#75111mypy 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: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