Needs work
Project:
AmazonS3
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
18 Jul 2013 at 00:10 UTC
Updated:
5 May 2015 at 13:31 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
pdrake commentedComment #2
djake commentedI'm very interested in this issue as well.
My sense is that this is somewhat outside the scope of this module, and the blame really lies with the modules that fail to properly implement stream wrappers. I posted #2049627: Support Remote Stream Wrappers on the ctools issue queue. I belive its caching mechanism hard codes the public scheme and that is the cause of the problem. It should use the default scheme or allow the user to choose.
This is a dealbreaker for using aggregation with multiple front ends without NAS. I want to use S3 and not NAS. If others are not having this problem, can you advise how you get around it? I wonder if CDN and/or AggAdv take care of this, but have never tested them.
Comment #3
justafishI agree with djake, this does seem to be an issue that's outside the scope of this module. Do you have an example use case for this pdrake?
Comment #4
pdrake commentedMy example use-case would be wanting to store the CSS and JS aggregates created by core aggregation on S3 (as suggested by djake, this is required to run multiple front end servers with S3 as the only shared filesystem). D7 core hard-codes the storage of these aggregates to use the public:// stream. hook__stream_wrappers_alter exists for just such a case, so that one can alter the definition of an existing stream (in this case, public://). Can you clarify for me what part of this patch you feel is outside the scope of the module?
Comment #5
justafish@pdrake Ahh I see now. Very cool! I've tested the patch and it does cause some things to break e.g. in the admin area there's no icons because now it tries to retrieve it from http://mybucketurl.amazons3.com/themes/seven/images/list-item.png
Comment #6
ivnate commentedI need this also. Use case: Running Drupal 7 on Amazon Opsworks, where the app servers are ephemeral and there isn't a permanent, shared file system.
I've applied the patch and am testing, but have the same issue as @justafish iwth the admin area.
For my use case:
All user generated files that would have been in file_public_path should be put into the bucket using s3://sites/default/files (or whatever the path is)
All static files that are only changed with code changes in my GIT repo (like theme images) should be served from the local file system.
Comment #7
djake commentedI have the exact same issues as pdrake and IVNate and they are very frustrating and have severely limited my ability to build a resilient site with Drupal.
However, my point is that Drupal core provides a mechanism for Remote Stream Wrappers, including this one, and I don't think it is appropriate (however desirable in the short term), for individual modules to redefine how Stream Wrappers operate. In other words, I believe the problem is not that s3: isn't the public stream, but rather "D7 core hard-codes the storage of these aggregates to use the public:// stream." I think that "public://" needs to be changed to the default scheme (s3, in our case), and so a patch should be submitted there, not here.
In ctools/includes/css.inc, the ctools_css_cache function defines $path = 'public://ctools/css', which is why I posted an issue to the ctools forum (linked in my prior comment). In addition to css/js aggregation, panels uses ctools to generate css files, which have the same problem as we are having for aggregation. I honestly don't know where css aggregation happens, but I think the issue is the same: the module (or core) should be using the default schema, rather than automatically assuming the public schema. Or, Drupal core should be changed to define the public schema as the default scheme, as this patch effectively does, but that should be a change in core, not this module.
I believe #1912138: Use S3 as the default file system- Not working raises a similar issue.
Fundamentally, I think Core assumes the public file stream is on the local filesystem. This seems to be a poor assumption in today's cloud based world, and hopefully will not be the case in Drupal 8, or with a patch to Drupal 7, but in the meantime, I think that individual modules (or components of core), simply need to change their code to implement stream wrappers, rather than assuming that I want to use the public scheme.
Comment #8
deviantintegral commentedI see a few issues with this patch as-is:
Anyways, if we can address all of these I'd be interested in getting this in. However, I'm guessing to do this it a way that doesn't break all the things will have to wait for D8.