Merge lp:~mnordhoff/loggerhead/yui-cdn into lp:loggerhead

Proposed by Matt Nordhoff
Status: Merged
Merged at revision: 324
Proposed branch: lp:~mnordhoff/loggerhead/yui-cdn
Merge into: lp:loggerhead
Diff against target: None lines
To merge this branch: bzr merge lp:~mnordhoff/loggerhead/yui-cdn
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Martin Albisetti Pending
Review via email: mp+5119@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Matt Nordhoff (mnordhoff) wrote :

Hi,

This adds a "--yui-cdn" option to serve-branches that will make Loggerhead serve the YUI files from Yahoo's CDN on http://yui.yahooapis.com/ instead of its local copy.

I added a yui_url method to BranchWSGIApp, similar to static_url, only...for the YUI files.

I tried it and it works, but that's about it.

Potential issues:

* Where should I have added the argument to BranchWSGIApp.__init__? I put it at the end. Should I have done that at all? Should I have used the 'config' argument or something instead?

* Whenever Loggerhead's copy of YUI is updated to a new version, the URL in BranchWSGIApp.yui_url will also need to be updated. Is anybody going to remember? :P

* I didn't add it to start-loggerhead. Do we care about that anymore?

* I didn't add any tests. To tell the truth, I lost interest while figuring out the config stuff. :P

* When Google's CDN gets a copy of YUI 3, I'll want to add support for that. I'd probably do it by changing the "--yui-cdn" argument to "--yui-cdn={google,yahoo,none}", which would break backwards compatibility. Do you want me to change it to "--yui-cdn={yahoo,none}" now so that won't be an issue?

* It still loads the individual files when using Yahoo!'s CDN. It would be more efficient to load the combined file, but, well, that would take more code. :P

Hmm, I think that's like 2 issues for every line of code I added. Maybe it's not worth it. :P

Revision history for this message
Martin Albisetti (beuno) wrote :

I actually really like this idea.
The only weird thing is that we're hard-coding the URL for yahoo in python itself. I can't really think of a much better way though, so I'm inclined to approve this.
Let's see if Michael es as crazy as I am :)

Revision history for this message
Michael Hudson-Doyle (mwhudson) wrote :

Looks fine to me.

review: Approve
Revision history for this message
Matt Nordhoff (mnordhoff) wrote :

(I sent this via email 20 minutes ago, but LP seems to have ignored it, so I'll add it via the web now. Sorry if it results in a duplicate.)

There's a new version up (though LP hasn't mirrored it yet):

<http://bzr.mattnordhoff.com/bzr/loggerhead/yui-cdn>
<http://bzr.mattnordhoff.com/loggerhead/loggerhead/yui-cdn/changes>
<http://bzr.mattnordhoff.com/loggerhead/loggerhead/yui-cdn/revision/323>
<http://bzr.mattnordhoff.com/loggerhead/loggerhead/yui-cdn/revision/324>

Upside:

* When using the YUI CDN, it'll load one combined script instead of N individual ones.

* The BranchWSGIApp.yui_url function is gone.

Downside:

* It's evil and depraved.

Revision history for this message
Matt Nordhoff (mnordhoff) wrote :

And I meant to add:

You can really pick any of the 3 different versions. They all work; they're just varying levels of different kinds of evil. :)

lp:~mnordhoff/loggerhead/yui-cdn updated
323. By Matt Nordhoff

Get rid of BranchWSGIApp.yui_url. Use YUI's combined JS file when using the CDN.

This is uglier, but it's better to use the single file (probably).

324. By Matt Nordhoff

Disgusting, evil template code to avoid repeating the list of YUI modules.

325. By Matt Nordhoff

Simplify the markup slightly

326. By Matt Nordhoff

Revert back to the simpler version with the yui_url method

327. By Matt Nordhoff

Rename --yui-cdn and use_yui_cdn to --use-cdn and use_cdn, respectively

328. By Matt Nordhoff

Badly-written NEWS entry. :D

Updating diff...

An updated diff will be available in a few minutes. Reload to see the changes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'loggerhead/apps/branch.py'
--- loggerhead/apps/branch.py 2009-03-17 00:02:21 +0000
+++ loggerhead/apps/branch.py 2009-04-01 20:11:48 +0000
@@ -31,7 +31,7 @@
3131
32 def __init__(self, branch, friendly_name=None, config={},32 def __init__(self, branch, friendly_name=None, config={},
33 graph_cache=None, branch_link=None, is_root=False,33 graph_cache=None, branch_link=None, is_root=False,
34 served_url=_DEFAULT):34 served_url=_DEFAULT, use_yui_cdn=False):
35 self.branch = branch35 self.branch = branch
36 self._config = config36 self._config = config
37 self.friendly_name = friendly_name37 self.friendly_name = friendly_name
@@ -42,6 +42,7 @@
42 self.graph_cache = graph_cache42 self.graph_cache = graph_cache
43 self.is_root = is_root43 self.is_root = is_root
44 self.served_url = served_url44 self.served_url = served_url
45 self.use_yui_cdn = use_yui_cdn
4546
46 def get_history(self):47 def get_history(self):
47 _history = History(self.branch, self.graph_cache)48 _history = History(self.branch, self.graph_cache)
@@ -79,6 +80,13 @@
79 def static_url(self, path):80 def static_url(self, path):
80 return self._static_url_base + path81 return self._static_url_base + path
8182
83 def yui_url(self, path):
84 if self.use_yui_cdn:
85 base = 'http://yui.yahooapis.com/3.0.0pr2/build/'
86 else:
87 base = self.static_url('/static/javascript/yui/build/')
88 return base + path
89
82 controllers_dict = {90 controllers_dict = {
83 '+filediff': FileDiffUI,91 '+filediff': FileDiffUI,
84 '+revlog': RevLogUI,92 '+revlog': RevLogUI,
8593
=== modified file 'loggerhead/apps/filesystem.py'
--- loggerhead/apps/filesystem.py 2009-03-31 19:31:45 +0000
+++ loggerhead/apps/filesystem.py 2009-04-01 20:11:48 +0000
@@ -32,7 +32,8 @@
32 branch_app = BranchWSGIApp(32 branch_app = BranchWSGIApp(
33 branch, name,33 branch, name,
34 {'cachepath': self._config.SQL_DIR},34 {'cachepath': self._config.SQL_DIR},
35 self.root.graph_cache, is_root=is_root)35 self.root.graph_cache, is_root=is_root,
36 use_yui_cdn=self._config.get_option('yui_cdn'))
36 return branch_app.app37 return branch_app.app
3738
38 def app_for_non_branch(self, environ):39 def app_for_non_branch(self, environ):
3940
=== modified file 'loggerhead/config.py'
--- loggerhead/config.py 2009-04-01 15:56:24 +0000
+++ loggerhead/config.py 2009-04-01 20:11:48 +0000
@@ -10,6 +10,7 @@
10 user_dirs=False,10 user_dirs=False,
11 show_version=False,11 show_version=False,
12 log_folder=None,12 log_folder=None,
13 yui_cdn=False,
13 )14 )
14 parser.add_option("--user-dirs", action="store_true", dest="user_dirs",15 parser.add_option("--user-dirs", action="store_true", dest="user_dirs",
15 help="Serve user directories as ~user.")16 help="Serve user directories as ~user.")
@@ -35,6 +36,8 @@
35 type=str, help="The directory to place log files in.")36 type=str, help="The directory to place log files in.")
36 parser.add_option("--version", action="store_true", dest="show_version",37 parser.add_option("--version", action="store_true", dest="show_version",
37 help="Print the software version and exit")38 help="Print the software version and exit")
39 parser.add_option('--yui-cdn', action='store_true',
40 help="Serve YUI from Yahoo!'s CDN")
38 return parser41 return parser
3942
4043
4144
=== modified file 'loggerhead/templates/macros.pt'
--- loggerhead/templates/macros.pt 2009-04-01 14:40:05 +0000
+++ loggerhead/templates/macros.pt 2009-04-01 20:11:48 +0000
@@ -17,32 +17,23 @@
17 var expanded_icon_path = <tal:block content="python:'\''+branch.static_url('/static/images/treeExpanded.png')+'\''" />;17 var expanded_icon_path = <tal:block content="python:'\''+branch.static_url('/static/images/treeExpanded.png')+'\''" />;
18 </script>18 </script>
19 <script type="text/javascript"19 <script type="text/javascript"
20 tal:attributes="src20 tal:attributes="src python:branch.yui_url('yui/yui-min.js')"></script>
21 python:branch.static_url('/static/javascript/yui/build/yui/yui-min.js')"></script>21 <script type="text/javascript"
22 <script type="text/javascript"22 tal:attributes="src python:branch.yui_url('oop/oop-min.js')"></script>
23 tal:attributes="src23 <script type="text/javascript"
24 python:branch.static_url('/static/javascript/yui/build/oop/oop-min.js')"></script>24 tal:attributes="src python:branch.yui_url('event/event-min.js')"></script>
25 <script type="text/javascript"25 <script type="text/javascript"
26 tal:attributes="src26 tal:attributes="src python:branch.yui_url('attribute/attribute-min.js')"></script>
27 python:branch.static_url('/static/javascript/yui/build/event/event-min.js')"></script>27 <script type="text/javascript"
28 <script type="text/javascript"28 tal:attributes="src python:branch.yui_url('base/base-min.js')"></script>
29 tal:attributes="src29 <script type="text/javascript"
30 python:branch.static_url('/static/javascript/yui/build/attribute/attribute-min.js')"></script>30 tal:attributes="src python:branch.yui_url('dom/dom-min.js')"></script>
31 <script type="text/javascript"31 <script type="text/javascript"
32 tal:attributes="src32 tal:attributes="src python:branch.yui_url('node/node-min.js')"></script>
33 python:branch.static_url('/static/javascript/yui/build/base/base-min.js')"></script>33 <script type="text/javascript"
34 <script type="text/javascript"34 tal:attributes="src python:branch.yui_url('anim/anim-min.js')"></script>
35 tal:attributes="src35 <script type="text/javascript"
36 python:branch.static_url('/static/javascript/yui/build/dom/dom-min.js')"></script>36 tal:attributes="src python:branch.yui_url('io/io-base-min.js')"></script>
37 <script type="text/javascript"
38 tal:attributes="src
39 python:branch.static_url('/static/javascript/yui/build/node/node-min.js')"></script>
40 <script type="text/javascript"
41 tal:attributes="src
42 python:branch.static_url('/static/javascript/yui/build/anim/anim-min.js')"></script>
43 <script type="text/javascript"
44 tal:attributes="src
45 python:branch.static_url('/static/javascript/yui/build/io/io-base-min.js')"></script>
46 <script type="text/javascript"37 <script type="text/javascript"
47 tal:attributes="src python:branch.static_url('/static/javascript/custom.js')"></script>38 tal:attributes="src python:branch.static_url('/static/javascript/custom.js')"></script>
48 <metal:block metal:define-slot="header_extras" />39 <metal:block metal:define-slot="header_extras" />

Subscribers

People subscribed via source and target branches