-
Notifications
You must be signed in to change notification settings - Fork 29
chore: Add async linting #968
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. 📢 Thoughts on this report? Let us know! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #968 +/- ##
=======================================
Coverage 96.03% 96.03%
=======================================
Files 827 827
Lines 19071 19071
=======================================
Hits 18315 18315
Misses 756 756
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
✅ All tests successful. No failed tests were found. 📣 Thoughts on this report? Let Codecov know! | Powered by Codecov |
ruff.toml
Outdated
| "E", # pycodestyle - error rules | ||
| "F", # pyflakes - general Python errors, undefined names | ||
| "I", # isort - import sorting | ||
| "PERF", # perflint - https://docs.astral.sh/ruff/rules/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe this should be "performance anti-pattern rules" and the rules link should be above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh woops, I misread what the original comment was trying to convey haha. Fixed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha no worries
ajay-sentry
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm! Happy to see we didn't have any errors for this guy either 😅
The Django postgres ORM is synchronous but the GraphQL resolvers are asynchronous. When any ORM commands are attempted to run directly within a resolver, this causes a runtime error
You cannot call this from an async context - use a thread or sync_to_async.I investigated different ways we could potentially use lint or type-check tools or even just a unit test that could catch these errors before they hit prod. Unfortunately I had trouble getting anything that could work reliably, and plug in with our existing tools (ruff, mypy, pytest). I explored the below:
So in the end I just added in the off-the-shelf async linter offered by flake8 that can catch common errors with async. Unfortunately it doesn't solve the original reason for this investigation re: Django ORM, but can at least be a backstop for other async related pitfalls.
I've also added this "runbook" for on-call or debugging response related to graphQL/async in
apiCloses codecov/engineering-team#2473