About the initial problem that started this thread, the associated bug number is #10948.
On Apr 6, 10:41 pm, Karen Tracey <kmtra...@gmail.com> wrote: > 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 -~----------~----~----~----~------~----~------~--~---
