Decouple 'uploaded/images' location from MEDIA_ROOT setting

Review Request #503 — Created Aug. 14, 2008 and discarded

Information

Review Board SVN (deprecated)

Reviewers

Adds 'UPLOADS_ROOT' configuration variable which decouples the 'uploaded/images' location from a being a suffix of MEDIA_ROOT.

This allows the upload directory to be overridden in local_settings.py, allowing multiple instances of Review Board on a system to share a system-wide installation containing the htdocs and media directories (ie. in the context of distributor packages).

For example, HTDOCS_ROOT and MEDIA_ROOT may point somewhere within /var/lib/ or /usr/share/, whilst a particular instance's UPLOAD_ROOT would point to (say) /srv/mysite.tld/uploads/

This change does not affect existing installations.

This diff additionally modifies the "chown" call to use ":" (colon) as the USER:GROUP seperator over the deprecated "." (period).
Tested locally with my (draft) Debian packages.
chipx86
  1. This looks good. There's a little bit more I'd like to see before this goes in.
    
    I'd like to see an UPLOADS_URL as well so that if people have some custom path for all uploaded files, they can say where it is. This will be important down the road if we support, say, Amazon S3 as a destination for uploads.
    
    Also not sure if I'd prefer UPLOAD_* or UPLOADS_*. I almost like UPLOAD_* better but I'm okay with either.
    1. I agree that would be a useful feature. However, looking at the code it seems that all users of djblets would need to depend on this additional setting - is that okay?
      
      (Looking at the UPLOADS_* diff vs UPLOAD_*, it looks much better so will change that next round)
    2. I may be confused. Why would UPLOAD_URL impact Djblets?
    3. I believe the `thumbnail` method in util/templatetags/djblets_images.py will need to be modified such that the UPLOAD_URL does not have to be under MEDIA_URL?
    4. Ah, I see. Yes, and crop_image. Both of these have problems with MEDIA_ROOT vs UPLOAD_ROOT anyway, though. They assume these images exist in MEDIA_ROOT and that may no longer be the case.
      
      Now for the thing that will really throw all this off: Django validates that uploaded files exist in MEDIA_ROOT, so UPLOAD_ROOT will never be able to be outside of MEDIA_ROOT. Which almost makes it pointless to have a custom variable. Maybe we should just document how to alias the path in the web server configuration...
      
      Thoughts?
    5. Urrgh, yes, I ACK that security feature.
      
      I'll have a think about this today and get back to you here. I'm very interested in the instancibility of Review Board, meaning that as little as possible needs to be copied or duplicated for each instance - I think the remarks in your comment imply that MEDIA_ROOT must be instance-specific, requiring symbolic links back inside that to the "system-wide" subdirectories of media. (Moving the current contents of media/ to a single subdirectory would be help a lot if this was the solution.)
      
      Any thoughts? I appreciate that this aspect may not be of any real use to you.
    6. I actually really want this too. I started work on a similar patch just before I saw that you posted this up. I didn't take into consideration that Django was doing this validation.
      
      I think if we can't figure out something clever here, we can always set up an Alias in the apache config for setting /media/uploaded to point to somewhere else.
      
      We're going to have this issue for more than just /media/uploaded. When extensions land post-1.0, we'll have another directory (/media/ext/) that the web server must be able to write to.
      
      Another option is to have individual installs set up their own htdocs, creating symlinks to the rb/yui/yui-ext/djblets directories, and providing their own uploaded and ext directories.
      
      I'm working on a script for making initial installation really easy. It could handle all this. Of course, on Windows we'd have to copy the files. Or maybe we could just provide a .htaccess that aliases the directories we care about to the system installs.
    7. So what I've ended up doing was implementing a script to maintain Review Board site trees. This handles installation and upgrades, and basically maintains the htdocs/media/ directory structure (symlinking when possible, erasing/copying the directories on Windows).
      
      What this boils down to is that uploaded/ is now its own directory inside the site's htdocs/media/ directory, and everything else is a symlink to a system-installed directory. So this change and the problems we discovered shouldn't really be relevant anymore. As such, I'm discarding this, and hopefully the new script will just work well enough for people that they don't run into any upload issues anymore.
  2. /trunk/reviewboard/templates/admin/manual-updates/media-upload-dir.html (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Should be UPLOADS_ROOT.
  3. 
      
Loading...