Code

#20355 closed New feature (wontfix)

How about pre/post_change_file signals to allow apps to delete replaced files?

Reported by: pdc@… Owned by: nobody
Component: File uploads/storage Version: 1.5
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Some people are surprised to discover that saving a new file to a FileField does not delete the old file, and deleting a model instance does not delete its dependent files. This is for a good reason of course—I may know that user avatars are disposable, say, but Django does not. But if the app does know whether old files can e disposed of, there does not seem to be an easy way to achieve this systematically because in pre_save and post_save signal handlers we do not have acces to both the old and new path names.

Would it be easy to have a pre_change_file and post_change_file signal pair, fired during the point in saving an instance when the files are really written? Its args would be something like the following:

 sender, instance, field_name, old_path, new_path, **kwargs

where old_path is None if the file is being saved for the first time, and new_path is None if the file is being deleted (in other words, replaced with no file).

As well as enabling app-specific decisions to be me made about deleting old files, this would also be a hook to hang the recalculation of ImageField width and height attributes that would avoid recalculating them needlessly.

Attachments (0)

Change History (3)

comment:1 Changed 11 months ago by mbrochh@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Can't you use pre_save and get the just to-be-saved model from the DB? Then you can compare old path against new path.

comment:2 Changed 10 months ago by akaariai

Note that deleting the file on the moment the change happens isn't safe. What if a rollback happens? Even at rollback time this isn't safe - there is still the possibility there are other transactions open seeing the old file, but at least that is a temporary error state.

So, I think queuing the file deletes on transaction.commit is needed, otherwise this is a very dangerous thing to do. So, I am marking this as wontfix, we can revisit this once transaction.post_commit hook is available...

comment:3 Changed 10 months ago by akaariai

  • Resolution set to wontfix
  • Status changed from new to closed

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.