-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
feat(server): conditionally run facial recognition nightly #11080
Conversation
aad1512
to
497c689
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.
LGTM
this.repository.getLatestFaceDate(), | ||
]); | ||
|
||
if (state?.lastRun && latestFaceDate && state.lastRun > latestFaceDate) { |
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.
Is this a string compare? Does that even work?
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.
Yes, since they're both ISO strings.
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.
I tested by hardcoding nightly: true
. The first time it ran (since there was no system metadata entry) and the second time it skipped.
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.
Ok. Maybe we should add a comment or something. I think it isn't very obvious, which makes it an easy place to introduce a bug.
Description
The facial recognition nightly currently runs every night, but there's no need when no new face has been detected since the last run.
This PR stores the last run date in the system metadata table and compares it with the newest
facesRecognizedAt
value in the job status table. It only does this in the nightly; manual runs still behave like before.The query for getting the latest face detection date is a full table scan, but it isn't worth indexing the column just for one non-interactive query a day. It at least avoids sorting.
Resolves #11078