Skip to content

Conversation

@mdmohsin7
Copy link
Member

No description provided.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds the functionality to delete a user's private cloud sync recordings from Google Cloud Storage when their account is deleted. This is achieved by adding a new function delete_all_user_private_cloud_sync_data and calling it within the delete_user_data flow.

The review comment suggesting the use of a batch operation for deleting files from GCS is valid and aligns with best practices for efficiency, especially for users with a large amount of data. This comment has been kept as is.

Comment on lines +411 to +419
# Delete all chunks for this user
chunks_prefix = f'chunks/{uid}/'
for blob in bucket.list_blobs(prefix=chunks_prefix):
blob.delete()

# Delete all merged audio files for this user
audio_prefix = f'audio/{uid}/'
for blob in bucket.list_blobs(prefix=audio_prefix):
blob.delete()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The current implementation deletes blobs one by one, which results in a separate API call for each file. This is inefficient and can be very slow for users with a large number of files, potentially leading to timeouts.

Using storage_client.batch() will group these deletions into fewer, more efficient multipart HTTP requests, making the process much faster and more reliable.

Suggested change
# Delete all chunks for this user
chunks_prefix = f'chunks/{uid}/'
for blob in bucket.list_blobs(prefix=chunks_prefix):
blob.delete()
# Delete all merged audio files for this user
audio_prefix = f'audio/{uid}/'
for blob in bucket.list_blobs(prefix=audio_prefix):
blob.delete()
# Use a batch request to efficiently delete all blobs for the user.
with storage_client.batch():
# Delete all chunks for this user
chunks_prefix = f'chunks/{uid}/'
for blob in bucket.list_blobs(prefix=chunks_prefix):
blob.delete()
# Delete all merged audio files for this user
audio_prefix = f'audio/{uid}/'
for blob in bucket.list_blobs(prefix=audio_prefix):
blob.delete()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants