-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix(server): do not process faces of deleted assets #6710
Conversation
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.
Why is this required?
It sounds like this is probably caused by trashed assets. So we should probably just make sure we ignore deleted assets instead. |
That was the original PR, but then I noticed that there actually is a |
Although I'm not sure why a row still exists if |
Right, I think it would be good to reproduce the actual issue before making a change like this. I have seen owner being null before, specifically when something references a relation that has been deleted. |
21bed63
to
ff7c6eb
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 great. Can you add a unit test for this still working with deleted assets?
Description
When querying for faces during recognition, a face from an asset that was deleted after queueing will throw an error. While TypeORM has a default filter for deleted entities, it still returns an entity if it has deleted relations. The quick fix is to check the asset relation. Moving forward, it may be good to switch to inner joins for relations. This is beyond the scope of this fix as it involves switching to query builders, complicates custom selects, etc.
Fixes #6709