fix(hc): Introduce Organization.get_bulk_owner_profiles #68756
Conversation
Support a specialized optimization in getsentry. The purpose is to get a lightweight view of the owners of an arbitrary number of organizations and while making only one RPC.
github-actions
bot
added
the
Scope: Backend
|
For context, see https://github.com/getsentry/getsentry/pull/13614 and https://github.com/getsentry/getsentry/pull/13622. See https://github.com/getsentry/getsentry/pull/13623 for the intended usage. This is a significant amount of code being added for an unfortunately narrow purpose, but ideally it will be the silver bullet we need to solve painfully slow page loads in |
src/sentry/models/organization.py
Outdated
| ) -> dict[int, RpcUserContact]: | ||
| owner_id_table = cls._get_bulk_owner_ids(organizations) | ||
|
|
||
| contacts = user_service.get_bulk_contact_info(user_ids=list(owner_id_table.values())) |
There was a problem hiding this comment.
This safe to roll out at the same time as the new service method, because nothing is going to hit get_bulk_owner_contacts until https://github.com/getsentry/getsentry/pull/13623 is merged.
Bundle ReportBundle size has no change ✅ |
|
Unit tests to be added, btw. |
| @@ -283,6 +287,10 @@ def get_user_avatar(self, *, user_id: int) -> RpcAvatar | None: | |||
|
|
|||
| return serialize_user_avatar(avatar=possible_avatar) | |||
|
|
|||
| def get_bulk_contact_info(self, *, user_ids: list[int]) -> list[RpcUserContact]: | |||
| users = User.objects.filter(id__in=user_ids).values_list("id", "name", "email") | |||
| return [RpcUserContact(id=id, name=name, email=email) for (id, name, email) in users] | |||
There was a problem hiding this comment.
I wonder if we have other flows in the application that could benefit from loading less data when needing to show lists of users 🤔
There was a problem hiding this comment.
Attempted to generalize this in #68847.
src/sentry/models/organization.py
Outdated
| return owner_id_table | ||
|
|
||
| @classmethod | ||
| def get_bulk_owner_contacts( |
There was a problem hiding this comment.
It would be good to have a test for this method as it is public interface that getsentry will rely on.
Offer a more performant alternative to UserService.get_many that provides all attributes that can be provided without fetching from any related tables.
Rename `ids_only` to more accurately reflect what it does, since `get_many_profiles` is now using it to fetch more than just IDs but still wants to avoid selecting any related data. Make `query_many` no longer private by convention. We now want to call it directly, so that we can then do a different kind of serialization than _UserFilterQuery's default. (DatabaseBackedUserSocialAuthService also was previously calling it directly.)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #68756 +/- ##
===========================================
- Coverage 91.22% 79.73% -11.50%
===========================================
Files 2848 6444 +3596
Lines 176397 286150 +109753
Branches 31461 49304 +17843
===========================================
+ Hits 160910 228148 +67238
- Misses 15486 57634 +42148
- Partials 1 368 +367
|
|
Looks good once #68847 is deployed. |
RyanSkonnord
changed the title
|
Should now be ready to merge. |
Support a specialized optimization in getsentry. The purpose is to get a lightweight view of the owners of an arbitrary number of organizations and while making only one RPC.
Support a specialized optimization in getsentry. The purpose is to get a lightweight view of the owners of an arbitrary number of organizations and while making only one RPC.