search

caktus / django-sticky-uploads

Enhanced file input widget for Django which uploads the file in the background and retains value on form errors.
BSD 3-Clause "New" or "Revised" License
32 stars 9 forks source link

Make a way to configure the base upload directory #21

Open Springsteen opened 8 years ago

Springsteen commented 8 years ago

It would be great if there is a way to configure the base upload directory it situations where multiple applications are writing in the "tmp" folder.

mlavin commented 8 years ago

With the change from #19 the prefix argument could be passed to mkdtemp to make these directories easy to find/clean up. Looking through my current /tmp/ folder that appears to be the approach of npm, pip, and others.

mlavin commented 8 years ago

Django already has a FILE_UPLOAD_TEMP_DIR setting as well. That could be used here in place of adding a new setting. Though this isn't relative to the system temp. Seems like this could be easy to misconfigure.

Springsteen commented 8 years ago

With the change from #19 the prefix argument could be passed to mkdtemp to make these directories easy to find/clean up. Looking through my current /tmp/ folder that appears to be the approach of npm, pip, and others.

If I understood you correctly you are suggesting to make the folder names something like "myproject_13ADsfa" ? I think that my decision is better because when you are cleaning up the tmp you will have to delete just one base folder, not many. Sorry if I misunderstood you.

Django already has a FILE_UPLOAD_TEMP_DIR setting as well. That could be used here in place of adding a new setting. Though this isn't relative to the system temp. Seems like this could be easy to misconfigure.

I agree with you that using this setting will be misleading, maybe just renaming the BASE_URL setting i'm using will be ok.

mlavin commented 8 years ago

If I understood you correctly you are suggesting to make the folder names something like "myproject_13ADsfa" ...

Yes that would be the result of my suggestion. While having multiple directories makes them slightly harder to clean up it also makes them harder to guess the location of the temporary file.

...maybe just renaming the BASE_URL setting i'm using will be ok.

Sorry no that's not exactly what I was saying. I'm saying that maybe this should use the existing setting FILE_UPLOAD_TEMP_DIR though I still have doubts as to if that is a good idea or not. I will say that any PR which introduces a new setting is unlikely to be merged.

Springsteen commented 8 years ago

Ok, I will refactor it like you suggest.