Skip to content
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

Only fetch fields we need in task manager #12045

Merged
merged 1 commit into from
Apr 13, 2022

Conversation

kdelee
Copy link
Member

@kdelee kdelee commented Apr 13, 2022

By using .only we select fewer columns, avoiding potentially large
fields that we never reference.

Also, small tweak to eliminate what was a duplicate dictionary of
hostname:instance, because we don't need build and carry two copies of
the same data.

SUMMARY

Jobs have lots of fields, but we only need a few of them for the task manager.

at @AlanCoding 's suggestion and after reading docs from Django and others, this looks like a better practice because NOT using .only or .defer we end up with what is essentially a SELECT * FROM table.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME
  • API
ADDITIONAL INFORMATION

docs:
https://docs.djangoproject.com/en/4.0/ref/models/querysets/#only

other links:
https://redwerk.com/blog/top-8-mistakes-when-working-with-django-orm/
https://testdriven.io/tips/815d1399-b284-4732-893e-deedd2aa8d49/

By using .only we select fewer columns, avoiding potentially large
fields that we never reference.

Also, small tweak to eliminate what was a duplicate dictionary of
hostname:instance, because we don't need build and carry two copies of
the same data.
Copy link
Contributor

@jbradberry jbradberry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I enthusiastically endorse.

@kdelee
Copy link
Member Author

kdelee commented Apr 13, 2022

downstream tests looking good, going to merge. thanks @jbradberry !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants