Process deleting projects as background task #1060
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1. Fix Default Filtering in ProjectFilterSet: - Modified the `ProjectFilterSet` to include `is_marked_for_deletion` in the default filtering when `is_archived` is not provided. - This ensures that projects marked for deletion are excluded from default views. 2. Migration for `is_marked_for_deletion`: - Added a migration (`0052_project_is_marked_for_deletion.py`) to introduce the `is_marked_for_deletion` field in the `Project` model. 3. Updated `Project` Model: - Added `is_marked_for_deletion` field to the `Project` model with a default value of `False`. 4. Modified `ProjectListView` to Exclude Marked Projects: - Modified the `get_queryset` method in `ProjectListView` to only fetch projects with `is_marked_for_deletion=False`. 5. Introduced `mark_for_deletion` Method: - Added a `mark_for_deletion` method in the `Project` model to set the `is_marked_for_deletion` flag and save the model. 6. Add `delete_in_background` Method in `Project` Model: - Introduced a new `delete_in_background` method that marks the project for deletion and enqueues a background deletion task. 7. Background Deletion Task: - Created a background deletion task using `django_rq` to handle project deletion asynchronously. - Checks if the project is still marked for deletion before proceeding. - If an error occurs during deletion, updates project status and logs an error. 8. Updated `ProjectActionView` Success Message: - Modified the success message in `ProjectActionView` for the "delete" action to provide a more informative message based on the count. Fixes: nexB#1002 Signed-off-by: Jayanth Kumar <jknani111@gmail.com>
jayanth-kumar-morem
force-pushed
the
1002
branch
from
6b5ca98
to
dc7b4b4
Compare
tdruez
requested changes
Jan 30, 2024
|
This is a good approach and well explained... it is likely missing not much to get this merged... @jayanth-kumar-morem do you mind looking into some of the comments? |
jayanth-kumar-morem
force-pushed
the
1002
branch
from
d6f4c43
to
5136eaf
Compare
- Remove comments - Use django rq - Use update instead of save Signed-off-by: Jayanth Kumar <jknani111@gmail.com>
jayanth-kumar-morem
force-pushed
the
1002
branch
from
5136eaf
to
bb312f7
Compare
tdruez
requested changes
Feb 27, 2024
|
|
||
| active_count = Project.objects.filter(is_archived=False).count() | ||
| active_count = Project.objects.filter( | ||
| is_archived=False, is_marked_for_deletion=False |
There was a problem hiding this comment.
We should use active() here as well, no?
|
|
||
| def delete_in_background(self): | ||
| # Mark the project for deletion and enqueue background deletion task | ||
| self.mark_for_deletion() |
There was a problem hiding this comment.
I think we can put self.update(is_marked_for_deletion=True) directly here, no need for the mark_for_deletion method yet since it's a one-liner that is only used in one place.
| self.update(is_marked_for_deletion=True) | ||
|
|
||
| def delete_in_background(self): | ||
| # Mark the project for deletion and enqueue background deletion task |
There was a problem hiding this comment.
A docstring would be better than a comment.
| project.delete() | ||
| except Exception as e: | ||
| info(f"Deletion failed: {str(e)}", project.pk) | ||
| project.update(is_marked_for_deletion=True) |
There was a problem hiding this comment.
I'm wondering is this line is of any use, the is_marked_for_deletion must be already True to reach this line, no?
Merge remote-tracking branch 'upstream/main' into 1002
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix Default Filtering in ProjectFilterSet:
ProjectFilterSetto includeis_marked_for_deletionin the default filtering whenis_archivedis not provided.Migration for
is_marked_for_deletion:0052_project_is_marked_for_deletion.py) to introduce theis_marked_for_deletionfield in theProjectmodel.Updated
ProjectModel:is_marked_for_deletionfield to theProjectmodel with a default value ofFalse.Modified
ProjectListViewto Exclude Marked Projects:get_querysetmethod inProjectListViewto only fetch projects withis_marked_for_deletion=False.Introduced
mark_for_deletionMethod:mark_for_deletionmethod in theProjectmodel to set theis_marked_for_deletionflag and save the model.Changed
deleteMethod inProjectModel:deletemethod todelete_actionin theProjectmodel to avoid conflicts.deletemethod that marks the project for deletion and enqueues a background deletion task.Background Deletion Task:
django_rqto handle project deletion asynchronously.Updated
ProjectActionViewSuccess Message:ProjectActionViewfor the "delete" action to provide a more informative message based on the count.Fixes: #1002