On Mon, Apr 6, 2009 at 1:56 PM, Marty Alchin <gulop...@gmail.com> wrote:
> > On Mon, Apr 6, 2009 at 1:16 PM, Karen Tracey <kmtra...@gmail.com> wrote: > > I feel like I'm going around in circles thinking about this one -- is > there > > a way out that someone else sees that I'm blind to? > > Welcome to my world! :( I spent a long time on issues like this prior > to getting the new file storage system in place at first, and > apparently skipped much of that step when doing r9766. At this point, > I think the most useful approach is to take a step back and just look > at the high-level options we have available. Trying to determine the > name in advance is a no-go, because that would just make the existing > known race conditions much more prominent. I see two options left: > > 1. Write something to disk (maybe the whole file, as it was before, > maybe just a placeholder, as the _save() does now) when the file is > first assigned, so that we have a full, proper filename early on. > Then, if the model doesn't get saved, somehow roll that back so we > don't leave stuff lingering on the filesystem. This is currently > something of a "then some magic happens" approach, since I'm not yet > sure if there's a reasonable, reliable way to roll back a file save. > But I'd like to keep an open mind, so there it is. This would also be > preferable if we can detect transaction rollbacks, because there is > also #6456 to consider. > I'm not seeing where exactly we have a _save() that writes just a placeholder? The _save() in django.core.files.storage.FileSystemStorage writes the whole content, and I don't see any other _save as opposed to save() methods in the FileField area? > > 2. Save the file all at once, as soon as possible, and blatantly > document this behavior when model validation goes in. Then, if people > need to validate a model that has a file on it, they have two options: > validate the model *before* saving the file to it, so the model is > known to be valid and can all be saved at once, or (if they need to > validate something in the file, such as its name or contents), add > their own code to delete the file if validation fails. Or, I suppose a > third option is to ignore the lingering files until they suck up too > much space, requiring a manual purge. > > I obviously hate to go with number 2, but if we can't come up with > something solid, I think it's the better approach, at least for now. > Documenting unfortunate behavior is certainly preferable to coding > even more unfortunate behavior. > What about: 3. Revert the removal of the FileField save_form_data override that was part of r9766. It is that removal that is causing save() on a ModelForm to no longer save the file to disk. If we restore it, names will get assigned for code using ModelForms just as they used to be. I am missing why the save_form_data override had to be removed in order to support the direct assignment of files to FileFields that is needed for model validation to work without the side-effect of model validation saving the file to disk. I can see that it might be preferable, in general, to delay saving as long as possible so you don't wind up with files written to disk for models that are ultimately not saved to the DB. But we didn't have that behavior in 1.0 -- model form save (even with commit=False), in 1.0.X, saves files to disk. Trying to move the save later at this point, for the model form usage scenario, leads to backwards-compatibility problems. It's kind of ugly to have one form of model creation/assignment ( via model forms) save data early and a 2nd (assignment of files to FileFields) save data late, so I can see we might not want to do this. But I thought I'd throw it out there. Reverting just that bit of r9766 also doesn't fix #10249 or #10300 which are resulting form the mixing in of FieldFile in the class bases. It also doesn't fix #10404 in the case where someone uses the new assign-file-to-FileField without saving the field explicitly before the model. So they'd still need fixing, but they seem more easily solved than this file name issue. Karen --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to django-developers+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~----------~----~----~----~------~----~------~--~---
