-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace sqlalchemy backref parameter with back_populates #2123
Conversation
e8966b1
to
ce4f7eb
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## chore/mypy-config-tests #2123 +/- ##
========================================================
Coverage 90.73% 90.74%
========================================================
Files 351 351
Lines 40904 40934 +30
Branches 8874 8871 -3
========================================================
+ Hits 37115 37146 +31
Misses 2481 2481
+ Partials 1308 1307 -1 ☔ View full report in Codecov by Sentry. |
ce4f7eb
to
7e3628b
Compare
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.
Looks good! 🔧
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.
The changes to tests/manager/api/controller/test_work.py don't seem strictly necessary, but I don't see any problem with them.
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.
Hmm... I forget why i changed this file... maybe for mypy? I'm not sure now, i guess i should have put a comment on this one.
Description
This PR updates all references to
backref
to useback_populates
as sqlalchmey recommends as the current best practice. This PR also tweaks some of the flags we use for mypy only in our tests, to disable some of the mypy warnings that aren't very meaningful for tests.Motivation and Context
The sqlalchemy
backref
parameter is considered legacy see: https://docs.sqlalchemy.org/en/14/orm/backref.html. Using this construct means that mypy (and IDE) can't infer the type of the dynamically generated parameter.This is part of the effort to get the code to a place where we can enable
check_untyped_defs
in mypy.How Has This Been Tested?
Checklist