Delete project and organization objects asynchronously #12541
Conversation
| """ | ||
| task_log = log.bind(model_name=model_name, object_pk=pk, user_id=user_id) | ||
| lock_id = f"{self.name}-{model_name}-{pk}-lock" | ||
| lock_expire = 60 * 60 * 2 # 2 hours |
There was a problem hiding this comment.
Maybe one hour is fine? or less...
There was a problem hiding this comment.
Yeah, I think it's not super important.
There was a problem hiding this comment.
Pull Request Overview
This PR converts project and organization deletion from synchronous to asynchronous operations to avoid request timeouts when deleting large objects. The changes introduce a new Celery task and view mixin to queue deletions for background processing.
Key Changes:
- Created
AsyncDeleteViewWithMessagemixin to queue deletions asynchronously - Added
delete_objectCelery task with locking to handle async deletions - Enhanced
set_change_reasonto support user tracking outside request context
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| readthedocs/core/tasks.py | Adds delete_object Celery task to asynchronously delete database objects with locking mechanism |
| readthedocs/core/mixins.py | Introduces AsyncDeleteViewWithMessage mixin that queues deletion tasks instead of directly deleting objects |
| readthedocs/core/history.py | Extends set_change_reason to accept optional user parameter for tracking deletions outside request context |
| readthedocs/projects/views/private.py | Updates ProjectDelete view to use async deletion with updated success message |
| readthedocs/organizations/views/private.py | Updates DeleteOrganization view to use async deletion with updated success message |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Looks good. Should we make a distinction in the UI when the project/organization is being deleted? It will be weird that you click on "Delete project", get redirected to the dashboard home page listing all your projects, and you still see the project there.
Is there an easy way to implement a UI distinction here?
| :param user_id: The ID of the user performing the deletion. | ||
| Just for logging purposes. | ||
| """ | ||
| task_log = log.bind(model_name=model_name, object_pk=pk, user_id=user_id) |
There was a problem hiding this comment.
Why are you using task_log here instead of log as we are doing in all the other places?
There was a problem hiding this comment.
log is a global variable and python gets confused with the scope, raising an error that the variable is unbound.
| """ | ||
| task_log = log.bind(model_name=model_name, object_pk=pk, user_id=user_id) | ||
| lock_id = f"{self.name}-{model_name}-{pk}-lock" | ||
| lock_expire = 60 * 60 * 2 # 2 hours |
There was a problem hiding this comment.
Yeah, I think it's not super important.
See #10040 (comment). We can, but we will need to change all querysets to ignore those objects... I guess we could do it incrementally, but hopefully the message "queued for deletion" hints users that the project is being deleted on the background. |
Yeah, this would be a ton of work. I don't think we want to ignore these projects, but at least just adding a visual differentiation in the UI. |
Closes #10040