-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
refactor: store logs as Redis hashes #823
Conversation
If the PR is accpeted, there should be a newer PR to drop the logging table. This would be implemented if all the logs are moved from SQL to Redis in order to avoid data loss. Thus, I have avoided the need to include a migration in this PR. |
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.
This looks great!
But I think we do need the migration in the same PR, because once this gets deployed, we will lose the logs from the past 7 days unless we migrate. We don't need to put the entire logging table into redis, just the logs from the past 7 days. Probably the easiest way to do it is as a YoYo migration, even though it doesn't actually touch the database. That is, we should not drop the logging table as part of the migration, that should be done as a separate manual step.
If you like, I can write the migration and append it to this PR. Just let me know. |
Okay. You can append it to the PR. |
Oh actually I don't think I can add to the PR because it's on a branch in your repo. I've opened #826 |
1c88d94
to
9bcd8e6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #823 +/- ##
==========================================
+ Coverage 91.55% 91.57% +0.02%
==========================================
Files 66 66
Lines 3575 3597 +22
==========================================
+ Hits 3273 3294 +21
- Misses 302 303 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Rationale
This PR implements the functionality to save logs to Redis. By setting expiry on the logs while saving, we can be sure that logs are not saved for years and there is no need for reliance on a cron job to delete old logs.
Changes
BaseWPOneDbTest
AND
queries for logsself.redis
Fixes
This fixes #778
This fixes #49