I would like to override the public:// file stream with the S3 file stream class in order to store and deliver certain files which are hard-coded to use public:// via S3 (such as CSS & JS aggregates).

Comments

pdrake’s picture

Status: Active » Needs review
StatusFileSize
new3.59 KB
djake’s picture

I'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.

justafish’s picture

I 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?

pdrake’s picture

My 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?

justafish’s picture

Status: Needs review » Needs work

@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

ivnate’s picture

I 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.

djake’s picture

I 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.

deviantintegral’s picture

Issue summary: View changes

I see a few issues with this patch as-is:

  1. It's possible to have two file records for the same file, under different paths. For example, public://bucket/image.jpg and s3://bucket/image.jpg. For managed files, this could lead to data corruption.
  2. We had to patch quite a few modules for the S3 wrapper to work properly, since it requires a bucket in the URL. I imagine a ton of modules hardcode public:// file paths without normalizing the URL.
  3. There's a performance issue in that S3 can be very slow to create new assets and do an initial stat. For image styles, we basically replaced the style generation with our own code, which is a pain to do for CSS and JS. And, we could have very similar problems with contrib modules.

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.