-
Notifications
You must be signed in to change notification settings - Fork 259
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: implement periodic full sync job to repair cache inconsistencies #10038
Conversation
TODO
|
44c5315
to
06fa8a8
Compare
71d22f5
to
ca3d6ce
Compare
67a12d7
to
c7dff93
Compare
33cc44d
to
3b577fe
Compare
89dd295
to
7f3418f
Compare
The menu looks like a candidate for a design review ;) Even without clear cache, because that's hidden unless debug enabled, it's cluttered. Do you think we should limit the repair command for a mailbox to run once per 10 minutes or so? (we could use the UserRateLimit attribute for it). It was rather quick even with my larger mailbox. |
Yeah, that is probably a good idea. Although I wasn't sure how to reflect this in the frontend. Disable the button while the rate limit is active via
I was able to confirm this using a very large test mailbox as well. The request itself should not cost that much processing time. It's comparable to a regular sync and we don't have a rate limit there. PS: I fully agree about the menu desperately requiring a redesign 😆 |
/backport to stable4.0 |
With the UserRateLimit attribute, the request will fail with a different status code. It would be nice to show a toast in that case, "hello, please wait 60 minutes before the next repair run". I don't think it's necessary to check that right away when building the menu or disable the button. Would it be possible to disable the button or show a loading animation while the request is running? |
I'll add a rate limit annotation and add a toast.
The button is disabled during the repair. I didn't add a loading animation though. |
Hmm, on second thought, this will limit repairing multiple distinct mailboxes. I ajdusted the rate limit to 3 attempts every 10 minutes. |
Sure! 10 attempts in 10 minutes is also okay. It's just to have a minimal protection against abusing the feature. |
Sorry, I overread that part. Also, okay for me to not apply a user rate limit if you think we don't need it. |
I'll add a more lax rate limit and then this is done. |
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.
Sorry to ask, but is it intended that everyone has the ability to approve changes? |
Yes, that is a GitHub feature. However, we have branch protections in place that enforce approvals by developers with write permissions before merging. That is why your check mark is shown greyed out. |
Oh, perfect, I was wondering, thanks for the clarification! :) |
Signed-off-by: Richard Steinmetz <[email protected]>
e4298a3
to
c0bed86
Compare
/backport to stable3.7 |
Run a background job once a week without any Horde cache to forcibly disable
QRESYNC
support. This should reconcile the UID list in our database.I also added a button to the mailbox action menu.
Close #9168