[bug] FileNotFoundError when trying to delete an image which links a non existing file #140
Comments
nemesifier
added
bug
|
Hey, can you brief me what is the issue to be fix? |
@prachipancholi what is it that is not clear in the issue description? |
|
I am working on this and have some doubts:
|
If the user intended to delete the FirmwareImage in the first place then I'm +1 for automatically deleting it. |
nemesifier
added
the
Hacktoberfest
|
Hey @nemesisdesign I think I can figure this one out! May I start working on it? |
|
I think it would be obvious that the FirmwareImage would have to be deleted, I mean, the original intention of the user was to delete it, now there's an exception, if the exception is catched the flow will resume normally and the image will be deleted. I have upgraded the issue description to reflect that. Anyone who wants to work on this, don't ask for permission, just send a PR! |
The change_view associated with BuildAdmin class was throwing
an exception when trying to delete a FirmwareImage object,
when the file associated with that object had already been
deleted from the file system.
There were two tasks according to my understanding:
1. Prevent the website to break and catch the error:
the return expression in change_view has been put
in a try/catch expression, with instructions/hints/warnings
for the user using message_user
2. If an image file has been deleted from the filesystem,
automatically remove the FirmwareImage assiciated with it:
The 'get_queryset' method for FirmwareImageInline class
has been over-written to automatically remove FirmwareImage objects
associated with deleted files and the information has been
logged with logger.
Fixes openwisp#140
…sp#140 I couldn't reproduce this issue with Django's standard `models.FileField` so I figured that the problem is with the way `private_storage` is being used. Strangely, there's no problem when delete method is called on an instance of the model. The error is always thrown when the delete method is called on a "modelForm" for an instance, the image file for which has been deleted. So the issue really is with the `private_storage` module. To get around the error though, what I have done is: - `try` to return change_view - `catch` the `FileNotFoundError` if it occurs - Get the `path` from the error itself - check if the specified directory exists, if not create one - create a temporary file - return `change_view` again (recursion) - This time the file is there to be deleted and the model instance is deleted as well This seems like a "brute force" solution, but since the origin of the issue is an external module, this is the only possible workaround (As we don't want to automatically delete any model instances because it would be strange for the user) Fixes openwisp#140
…sp#140 Added a try/catch block in the change_view of `BuildAdmin` class. If a `FileNotFoundError` is caught, the relavant file/directory will be temperorily re-created just before recursively calling change_view again. Fixes openwisp#140
…sp#140 Added a try/catch block in the return statement in `BuildAdmin` `change_view` If a `FileNotFoundError` is caught, the file/directory from the error path is recreated temperorily before recursively calling `change_view` again, to complete the delete operation without throwing any errors. Fixes openwisp#140
|
Hello, Is there anyone working on this issue if not then can you assign this issue to Me? |
|
I think this is fixed. I could not reproduce this issue. This is my repo version: commit 9c7840c (HEAD -> master, origin/master, origin/HEAD) |
|
Hey @nemesifier , is this issue is open or need to fix? |
|
@Ashish8329 I am not sure if this issue is still valid, testing if you can reproduce it would be useful. |
This happens mostly during testing, but may happen during migrations as well.
How to replicate:
Result:
Solution:
The text was updated successfully, but these errors were encountered: