chore(users): delete old worker code #1959
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd the label merge-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Deploying rivet with
|
| Latest commit: |
6de4e36
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://25cd0435.rivet.pages.dev |
| Branch Preview URL: | https://01-29-chore-users-delete-old.rivet.pages.dev |
ABCxFF
force-pushed
the
01-28-feat_user_migrate_pub_sub
branch
from
55df57b to
369b249
Compare
ABCxFF
force-pushed
the
01-29-chore_users_delete_old_worker_code
branch
from
d17acc2 to
967936e
Compare
ABCxFF
force-pushed
the
01-28-feat_user_migrate_pub_sub
branch
from
369b249 to
b3252bc
Compare
ABCxFF
force-pushed
the
01-29-chore_users_delete_old_worker_code
branch
from
967936e to
8b00f39
Compare
ABCxFF
force-pushed
the
01-28-feat_user_migrate_pub_sub
branch
from
b3252bc to
e372d28
Compare
ABCxFF
force-pushed
the
01-29-chore_users_delete_old_worker_code
branch
from
8b00f39 to
a082694
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR removes old worker code and migrates user-related functionality to a pub/sub architecture. The changes primarily involve cleanup and removal of legacy code.
- Removes entire
user-identityanduser-workerpackages including operations for user creation, deletion, profile management, and token handling - Removes worker tests for user operations including admin promotion, party updates, team member removal, and presence updates
- Updates import paths across multiple files by removing redundant leading double colons (
::), making paths relative instead of absolute - Removes
faker-userdependencies from various packages as part of consolidating fake data generation - Removes
user-avatar-upload-completeoperation and its associated tests, suggesting avatar functionality is being migrated
119 file(s) reviewed, 15 comment(s)
Edit PR Review Bot Settings | Greptile
packages/services/user/worker/src/workers/event_party_member_update.rs
Outdated
Show resolved
Hide resolved
ABCxFF
force-pushed
the
01-28-feat_user_migrate_pub_sub
branch
from
e372d28 to
51169c4
Compare
ABCxFF
force-pushed
the
01-29-chore_users_delete_old_worker_code
branch
from
a082694 to
6de4e36
Compare
ABCxFF
force-pushed
the
01-28-feat_user_migrate_pub_sub
branch
from
51169c4 to
016e8f5
Compare
ABCxFF
force-pushed
the
01-29-chore_users_delete_old_worker_code
branch
from
6de4e36 to
a69da5b
Compare
There was a problem hiding this comment.
PR Summary
Based on the provided context, I'll create a summary of the most recent changes that haven't been covered in previous reviews.
This PR continues the cleanup of old worker code and migration to pub/sub architecture, focusing on user-related functionality and code organization.
- Removes all user worker event handlers (
event_party_update.rs,event_team_member_remove.rs, etc.) that were responsible for user notifications and state updates - Removes the user profile validation operation (
profile-validate) which handled display name, account number, and bio validation rules - Removes the email resolution functionality (
resolve-email) that mapped email addresses to user IDs - Removes the team list operation (
team-list) that was responsible for fetching and aggregating team membership information - Removes placeholder test files that were never implemented, suggesting a more focused testing approach in the new architecture
The changes appear thorough and consistent with the migration to pub/sub, though careful verification of the new implementation's functionality will be important.
119 file(s) reviewed, 28 comment(s)
Edit PR Review Bot Settings | Greptile
| let identity = chirp_workflow::compat::op( | ||
| &ctx, | ||
| ::user::ops::identity::get::Input { | ||
| user::ops::identity::get::Input { | ||
| user_ids: vec![user_id] | ||
| }, | ||
| ) |
There was a problem hiding this comment.
style: consider using the op! macro here instead of chirp_workflow::compat::op for consistency with the rest of the codebase
| debug-email-res.workspace = true | ||
| user-identity-get.workspace = true | ||
|
|
There was a problem hiding this comment.
style: Consider removing the extra blank line at the end of the file for consistency
| region-resolve-for-game.workspace = true | ||
| token-create.workspace = true | ||
| token-revoke.workspace = true | ||
| user-identity-get.workspace = true | ||
| rivet-config.workspace = true |
There was a problem hiding this comment.
logic: Removing user-identity-get.workspace may break functionality that requires user identity verification in the matchmaker service. Verify that this dependency is not needed for any authentication or user validation flows.
There was a problem hiding this comment.
logic: The dev-dependencies on chirp-worker, chirp-workflow, and user suggest this package was used in testing. Check that any integration tests in other packages that depended on faker-user have been updated.
There was a problem hiding this comment.
style: Since this package is being removed from the workspace, ensure it's also removed from the workspace members list in the root Cargo.toml
There was a problem hiding this comment.
logic: the old code included cache purging for deleted users. Make sure this cache invalidation is still happening in the new implementation to prevent stale data
There was a problem hiding this comment.
style: the worker used batching for uploads (UPLOAD_BATCH_SIZE) and messages (MESSAGE_BATCH_SIZE) to handle large deletions efficiently. Ensure the new implementation has similar performance considerations
There was a problem hiding this comment.
logic: This worker was responsible for critical user profile updates. Before removing it, verify that equivalent functionality exists in the new pub/sub system, particularly the profile validation, cache purging, and analytics tracking.
There was a problem hiding this comment.
logic: The removal of this worker could break real-time party state synchronization if the replacement pub/sub system is not fully implemented. Party members may not receive updates about party changes.
There was a problem hiding this comment.
logic: This test was never implemented but its corresponding worker code in event_team_member_remove.rs still exists and handles team member removal events. Consider either implementing the test or removing the worker code as well if it's no longer needed.
Changes