Skip to content
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

fix: Purge user after deleting records that reference the user #2374

Closed

Conversation

fregataa
Copy link
Member

@fregataa fregataa commented Jul 4, 2024

Issue

GQL purge_user mutation does not work.

  1. The mutation spawns an asynchronous task to delete user's vfolders and immediately calls DELETE FROM users ... query, which violates a foreign key constraint set between vfolders.user and users.uuid.
  2. The mutation checks whether the user owns any running kernels, but it does not check "keypairs" the user owns have running kernels.

How it changes

  1. Delay DELETE FROM users ... query if the user owns vfolders where actual content remains. The mutation spawns an asynchronous task that calls DELETE FROM users ... query after finishing deletion of vfolders.
  2. Check the user's keypairs own any running sessions.

Note

The purpose of this PR is a hotfix not a refactoring or enhancement.
The changed behavior does not ensure any deletion of vfolders and as it has been, it does not show any failure of vfolder deletion.

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • API server-client counterparts (e.g., manager API -> client SDK)

@fregataa fregataa added the urgency:blocker IT SHOULD BE RESOLVED BEFORE NEXT RELEASE! label Jul 4, 2024
@fregataa fregataa added this to the 24.03 milestone Jul 4, 2024
@fregataa fregataa self-assigned this Jul 4, 2024
Copy link

graphite-app bot commented Jul 4, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “flow:merge-queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “flow:hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

@github-actions github-actions bot added comp:manager Related to Manager component size:L 100~500 LoC labels Jul 4, 2024
@fregataa fregataa force-pushed the fix/purge-user-after-deleting-all-user-references branch from b6bbbbb to feee137 Compare July 4, 2024 06:06
@fregataa fregataa force-pushed the fix/purge-user-after-deleting-all-user-references branch from aaabc4d to 1e4c934 Compare July 4, 2024 06:19
@fregataa fregataa changed the title fix: Delete vfolders in background when purging a user fix: Purge user after deleting records that reference the user Jul 4, 2024
@fregataa fregataa marked this pull request as ready for review July 4, 2024 06:56
@github-actions github-actions bot added comp:client Related to Client component comp:cli Related to CLI component labels Jul 4, 2024
@fregataa fregataa marked this pull request as draft July 5, 2024 14:17
@fregataa fregataa marked this pull request as ready for review July 5, 2024 15:05
@fregataa
Copy link
Member Author

fregataa commented Jul 9, 2024

Close this PR since #2404 will cover the same issue

@fregataa fregataa closed this Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:cli Related to CLI component comp:client Related to Client component comp:manager Related to Manager component size:L 100~500 LoC urgency:blocker IT SHOULD BE RESOLVED BEFORE NEXT RELEASE!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant