Needs review
Project:
Media Mover
Version:
5.x-1.0-rc7
Component:
User interface
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
21 Apr 2009 at 21:18 UTC
Updated:
17 Jun 2010 at 11:07 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
jthaxton commentedI have worked this out and will submit a patch once I have everything worked out.
Comment #2
struesda commentedSo what was the problem? I just bumped into this same thing...
Comment #3
arthurf commentedSo when you go to add a configuration, the settings for permissions on the S3 configuration has no options?
Comment #4
laken commentedSame issue for me - I'm using Media Mover 5.x-1.x-dev (2009-Sep-28) and I also a) get no options in the S3 permissions dropdown and b) get this PHP error:
[Thu Oct 01 13:20:45 2009] [warn] mod_fcgid: stderr: PHP Fatal error: Access to undeclared static property: S3::$perms in /home/clients/websites/andy/sites/mysite/modules/contrib-dev/media_mover/contrib/mm_s3/mm_s3.module on line 289Comment #5
laken commentedOk, issues appear to be unrelated. Working on a fix and will post patch when done.
Comment #6
laken commentedHere's a patch which fixes the permissions selector drop-down, fixes the fatal "Access to undeclared static property" error, and I think fixes the watchdog call near line 303, which was using the D6 API call (might have been accidentally backported).
For the watchdog, I say "I think" because I haven't been able to get S3 to throw an error which would be reported back, so haven't confirmed a proper error message is being put into watchdog correctly. Would love it if someone could test that part.
Comment #7
robert castelo commented#6 patch worked for me - thanks laken.
- Selector now has options.
- Media Mover can now connect to S3
Comment #8
arthurf commentedThanks for the patch. I took a slightly different approach to keep code a bit cleaner, but same concept. Note there are two other important changes in this commit:
* mm_s3 is now passing the full file path to amazon to prevent duplicate files
* mm_s3 is now passing the file mimetype in $s3->putObjectFile
Can you guys give this a review and see if it is working for you? Note that this was committed to 5.x 6.1x 6.2x
Comment #9
laken commentedArthur, thanks for the commit - here are some comments:
1) I think you accidentally left out the fix for the drop-down permissions selector - I've re-rolled that patch against the current code and attached it (D5 only)
2) Can you clarify the design motivations behind passing the full path to S3? Before this change, if I specified the bucket 'images' and had a file 'mymovie.flv', on S3 I would end up with:
With your change, I now have on S3:
Is that what you intend? This makes browsing my buckets in a client like S3Hub - which uses slashes in filenames to emulate 'virtual' subdirectories - more difficult. Could you explain the error case for duplicate filenames which prompted this change?
On my setup, this change breaks XSPF_playlist, which is still outputting
<location>http://s3.amazonaws.com/my_movies/mymovie.flv</location>rather than
<location>http://s3.amazonaws.com/my_movies/sites/mysite.org/files/media_mover/ffmpeg/converted/4/mymovie.flv</location>which is the correct URL to the file on S3. So my videos stopped playing! I haven't dug into the XSPF_playlist code, but I imagine there's a place where the full filepath would need to be built to match the new way you're saving to S3. I'm concerned that since this is committed, if people upgrade to the latest dev code it may break their sites. (Again, only testing on D5)
Looking forward to your feedback so we can get this working smoothly.
Comment #10
arthurf commentedI switched over to use the full drupal file path for the s3 file name to make it possible to use amazon as a CDN- this means that you can just rewrite your domain to s3 without having to worry about having duplication of file names. This also helps with directory layout. I suppose that this could be an option in the configuration- I'll look into doing that.
Asper xspf you may have cached media mover files- try truncating your cache_media_mover table
Comment #11
arthurf commentedI just backported a bunch of changes to the file path handling from D6
Comment #12
laken commentedThanks for the continued care on the 5.x branch here. Yesterday I updated to latest dev version and saw the improvements in how you're handling the permissions, nice! I also understand why it's valuable to have an option to store the complete drupal path on S3, and I'm using that option for my application.
However, I found some other problems, the most major one being that storage_file and complete_file values weren't being properly saved - this turned out to be a typo ($filename rather than $filepath on line 389.)
Also fixed several small errors, including correcting D6 watchdog calls (see patch attached).
One problem I'm not sure how to deal with: mm_s3 can't properly delete files from S3 anymore, if they're stored using full paths. Line 420 is the culprit:
This breaks with full paths b/c the full path gets used for the bucket name. I'm not sure how to fix this.
Could you review this patch and suggest a way to deal with the bucket issue?
Thanks!
Comment #13
laken commentedAnother thing I noticed in the latest release:
I'm using the cck/filefield harvesting and S3 for storage. With this latest dev code I noticed that nodes which had previously been processed and had their files moved to Amazon seemed to re-send their files to S3 when the configuration was run again, rather than just unprocessed nodes. This isn't good - as the # of nodes grows we'll have a lot of needless re-transferring to S3. But I'm not sure where to check the code to fix this.
Any ideas?
Comment #14
sukh.singh commentedHi,
I am using media_mover-5.x-1.0-rc7 version. After adding patch #6 everthings works well. But one more issue come to my notice. AS this module moves files to S3 bucket it does not maintain the directory structure of files folder for example image cache folder structure. What it real does is it store the images under one folder which called bucket in S3.
Let me know if this is bug or not.
Comment #15
sukh.singh commented