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

the delete functions in the models that use MEDIA_ROOT don't actually remove the files from the OS #15

Open
chrissprague opened this issue Apr 5, 2015 · 3 comments

Comments

@chrissprague
Copy link
Member

run through: create an exam review, and then delete it through the admin site

what will happen is that django properly recognizes that it is removed from the database, but the actual file is still in MEDIA_ROOT (csc_new/media/[media] - where, in this case, [media] would be "exam_reviews" -- no quotes)

not a big deal but we don't want our webserver getting clogged with unused files without us being aware

anyone have an idea on how to fix this? I'm guessing it's just a path reference issue (e.g. the name referred to in the delete code might be wrong). Or am I missing something/doing something wrong?

@chrissprague
Copy link
Member Author

Also, reminder that we need to go in and delete files that were orphaned in this way if my understanding was correct

@khipkin
Copy link
Member

khipkin commented Apr 10, 2015

@chrissprague, I figured out what the problem is!

According to the Django documentation here, "overridden model methods are not called on bulk operations".

When the admin site does pretty much anything, it does it as a bulk operation. So our custom delete() methods are never called, which is why our custom delete behavior never actually happens.

Instead of trying to override the default behavior, we should be hooking into the models' pre_delete signal(s). This signal is always sent during deletion, bulk or not, and (even better) we know that the object still exists in the database.

More info on pre_delete (which we should use) and post_delete (which we should not use).

...

I'm having trouble figuring out how to actually implement this solution, but this is what we have to do.

@chrissprague
Copy link
Member Author

@khipkin Nice catch! Thanks. 👍

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

No branches or pull requests

2 participants