Refactor IAM id access #8969
Refactor IAM id access #8969
Conversation
zhiltsov-max
requested review from
SpecLad
and removed request for
Marishka17
zhiltsov-max
force-pushed
the
zm/optimize-iam-data-access
branch
from
677cb5d to
c382cc4
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8969 +/- ##
===========================================
+ Coverage 73.82% 73.85% +0.03%
===========================================
Files 417 417
Lines 44583 44605 +22
Branches 4028 4031 +3
===========================================
+ Hits 32912 32942 +30
+ Misses 11671 11663 -8
|
cvat/apps/engine/permissions.py
Outdated
| @@ -456,7 +462,7 @@ def create(cls, request, view, obj, iam_context): | |||
| def create_scope_view(cls, request, task: Union[int, Task], iam_context=None): | |||
| if isinstance(task, int): | |||
| try: | |||
| task = Task.objects.get(id=task) | |||
| task = Task.objects.select_related("organization").get(id=task) | |||
There was a problem hiding this comment.
Could you explain this? It seems to go against the purpose of this PR, as it adds a new join.
There was a problem hiding this comment.
Did you mean select_related("project")?
There was a problem hiding this comment.
It was to remove 1 request from the following get_iam_context() -> get_organization() call, but probably it should be removed from this PR to keep it focused and simple enough. Prefetches will be in another PR.
| 'id': self.obj.organization.id | ||
| } if self.obj.organization else None | ||
| 'id': self.obj.organization_id | ||
| } if self.obj.organization_id else None |
There was a problem hiding this comment.
I would suggest to add a common function for this pattern, since it seems to be repeating a lot:
def id_dict(id):
if id is None: return None
return {'id': id}There was a problem hiding this comment.
Maybe it's a good idea, but it's out of the scope for this PR. It looks like these data structures should be described somehow on the server side.
|
The existing object id access pattern for foreign keys produces unnecessary dependencies on the whole foreign object. It can be optimized to using just the object id, which is stored in the same db row in most cases (except m2m fields). This change doesn't immediately lead to optimized DB requests, but it opens optimization opportunities in the future.
objfield type annotations forPermissiontypesMotivation and context
How has this been tested?
Checklist
developbranchLicense
Feel free to contact the maintainers if that's a concern.