-
Notifications
You must be signed in to change notification settings - Fork 142
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
✨ Send account events to ozone #708
base: main
Are you sure you want to change the base?
Conversation
Yup! This is roughly the shape I was thinking. Some notes:
|
More quick notes:
|
Thanks, these notes help a lot!
these are quite tricky since profiles may be updated within minutes because people are changing their bio or smth or accounts may be deactivated/reactivated because someone wanted to check how it works etc. I think just setting a shorter window for the last event check, like within the last 30s or smth would cover most legit cases?
I think we would cover this via record snapshots in ozone. automod would just emit the changes and ozone would keep snapshots so that we have full history for record updates. for account events, these don't matter much, do they? I guess for handle change it does but then we kinda have that history in plc, right? |
for dedupe: yup! you are right, we definitely want to capture all those edits. maybe we can de-dupe from the CID or timestamp in the most recent event, instead of just the event type? for "what changed": PLC is a good example. if only looking at PLC changes, the existing table works well: you can scan the columns and see what changed. in the ozone event history, it is probably going to be a mix of reports, comments, actions/labeling, and then a identity/PLC event in the middle. in that context is isn't easy to see what that one identity event represented though, would need to dig deeper. it is definitely an improvement to have it show up at all, and we can probably figure this out later in either the ozone backend or client. |
These are refactors to the `ozone-account-events` branch that we went through together in a pair-programming session. I haven't re-reviewed these yet, and there are a couple TODOs, but it at least compiles.
Summarizing recent changes
|
Resolved Conflicts: api/ozone/moderationqueryStatuses.go
Resolved Conflicts: cmd/hepa/main.go cmd/hepa/server.go
I was re-working this over in #910, but i'll just push that work here and close the other one. Summarizing changes made here:
And some outstanding issues: at least in staging, the de-dupe query doesn't seem to work. I get:
when using event types like another problem i'm seeing with "delete" record ops is that some ozone events require a "strong ref"; but with a deletion there is no CID for the record, so a strongref can't be created. |
Manually Resolved Conflicts: automod/engine/persist.go
One path forward with this PR is to split out the account/identity update stuff from the record-op update. The account/identity stuff should be good to go once the de-dupe "queryEvents" issue is resolved, and functionality is tested to work as expected. The recordOp stuff in the general case needs a resolution to the strongRef problem for deleted records, same as: #739 |
Specific features: