Closed (won't fix)
Project:
CDN
Version:
5.x-1.0-rc2
Component:
Module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
21 Jan 2008 at 01:17 UTC
Updated:
21 May 2009 at 16:39 UTC
I hope it is not too late to fix a major shortcoming with static file serving.
Offloading static contents to a dedicated fileserver or a CDN can significantly reduce Apache load and enable switching KeepAlive Off, greatly boosting server and network performance. This solution is superior to using a Reverse Proxy, as discussed here:
http://www.lullabot.com/articles/using-lighttpd-static-file-server
There is a simple D5 patch that wraps requests for static files into a new static_file function, which depending on configuration settings could use a static fileserver or request from a CDN. Here is the code and info:
http://drupal.org/node/149402#comment-699065
Could we get this into D6? This is a simple patch only affecting common.inc, theme.inc, file.inc and page.tpl.php
Comments
Comment #1
kbahey commentedWhile I support the feature, I am against it getting in this late. We are at RC2 already, and this is not a small patch.
Also, the JS aggregation part and the static file serving part needs to be separated, for easier reviews. There are also many extraneous changes in the patch (specially in .js files).
Comment #2
dkruglyak commentedOnce JS aggregation is removed this is a very small / simple patch. There are only two trivial changes:
1) Wrapping a handful of base_path uses in a new static_file function
2) Implementation of this function, that nicely degrades to base_path unless configuration is set
I could whip up the slimmed down patch if we can get this into D6. Otherwise we still need a D6 issue to track this patch and keep it updated in sync with D6 maintenance releases.
Would be a real shame if this patch does not get commited while D6 is still open.
Comment #3
moshe weitzman commentedsee http://drupal.org/node/207330.
Comment #4
catchmoshe - does that make this redundant then? Or keeping open for the js aggregation element?
Also, there's no patch here.
Comment #5
dkruglyak commentedLooks like the problem addressed well in D5 and D6 core.
Now this means that modules like CDN need to be updated to always output links via url() function instead of anything custom (e.g. file_url) and instead provide code templates for rewriting functions - according to directions from #3. I also think CDN module in D5 could use a simpler function http://api.drupal.org/api/function/custom_url_rewrite/5, that is invoked from url() through path functions.
That would also make the implementation consistent between D5 and D6.
Comment #6
wim leers1) url() is not appropriate for file URLs. See http://api.drupal.org/api/function/url/5. Hence file_url(), which makes sense, since it's for … well, file URL's.
2) Due to my first point, your suggestion cannot be used, since that only applies to files served through the url() function.
Now, if I put my clean separation argument aside, we could use the url() function and thus the custom_url_rewrite() function. But I believe there was a reason for not using it, although I can't remember it ATM. I'm in the middle of my exams, so I'm not going to go into further depth with this.
If you want this to progress quickly (I'm not going to have time to work on this module in the next 4 weeks at the least), please submit patches.
Comment #7
dkruglyak commentedNo rush, I just wanted to get the issue and my notes in the queue...
That is what I meant - a combination of url() and custom_url_rewrite(). This seems like a nice way to consolidate CDN module patches to use APIs available in core for both D5 and D6. If there is a reason not to do it this way - let's investigate.
Comment #8
wim leersI discussed this with drewish at DrupalCon DC and the conclusion was that we should use file_create_url() for creating all file URLs. file_create_url() should then allow a new function (
custom_file_url_rewrite()) to rewrite the file URLs.This is exactly what the new core patch does.