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

Proposed by Matt Nordhoff
Status: Merged
Merged at revision: 324
Proposed branch: lp://staging/~mnordhoff/loggerhead/yui-cdn
Merge into: lp://staging/loggerhead
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~mnordhoff/loggerhead/yui-cdn
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Approve
Martin Albisetti Pending
Review via email: mp+5119@code.staging.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. :)

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
1=== modified file 'loggerhead/apps/branch.py'
2--- loggerhead/apps/branch.py 2009-03-17 00:02:21 +0000
3+++ loggerhead/apps/branch.py 2009-04-01 20:11:48 +0000
4@@ -31,7 +31,7 @@
5
6 def __init__(self, branch, friendly_name=None, config={},
7 graph_cache=None, branch_link=None, is_root=False,
8- served_url=_DEFAULT):
9+ served_url=_DEFAULT, use_yui_cdn=False):
10 self.branch = branch
11 self._config = config
12 self.friendly_name = friendly_name
13@@ -42,6 +42,7 @@
14 self.graph_cache = graph_cache
15 self.is_root = is_root
16 self.served_url = served_url
17+ self.use_yui_cdn = use_yui_cdn
18
19 def get_history(self):
20 _history = History(self.branch, self.graph_cache)
21@@ -79,6 +80,13 @@
22 def static_url(self, path):
23 return self._static_url_base + path
24
25+ def yui_url(self, path):
26+ if self.use_yui_cdn:
27+ base = 'http://yui.yahooapis.com/3.0.0pr2/build/'
28+ else:
29+ base = self.static_url('/static/javascript/yui/build/')
30+ return base + path
31+
32 controllers_dict = {
33 '+filediff': FileDiffUI,
34 '+revlog': RevLogUI,
35
36=== modified file 'loggerhead/apps/filesystem.py'
37--- loggerhead/apps/filesystem.py 2009-03-31 19:31:45 +0000
38+++ loggerhead/apps/filesystem.py 2009-04-01 20:11:48 +0000
39@@ -32,7 +32,8 @@
40 branch_app = BranchWSGIApp(
41 branch, name,
42 {'cachepath': self._config.SQL_DIR},
43- self.root.graph_cache, is_root=is_root)
44+ self.root.graph_cache, is_root=is_root,
45+ use_yui_cdn=self._config.get_option('yui_cdn'))
46 return branch_app.app
47
48 def app_for_non_branch(self, environ):
49
50=== modified file 'loggerhead/config.py'
51--- loggerhead/config.py 2009-04-01 15:56:24 +0000
52+++ loggerhead/config.py 2009-04-01 20:11:48 +0000
53@@ -10,6 +10,7 @@
54 user_dirs=False,
55 show_version=False,
56 log_folder=None,
57+ yui_cdn=False,
58 )
59 parser.add_option("--user-dirs", action="store_true", dest="user_dirs",
60 help="Serve user directories as ~user.")
61@@ -35,6 +36,8 @@
62 type=str, help="The directory to place log files in.")
63 parser.add_option("--version", action="store_true", dest="show_version",
64 help="Print the software version and exit")
65+ parser.add_option('--yui-cdn', action='store_true',
66+ help="Serve YUI from Yahoo!'s CDN")
67 return parser
68
69
70
71=== modified file 'loggerhead/templates/macros.pt'
72--- loggerhead/templates/macros.pt 2009-04-01 14:40:05 +0000
73+++ loggerhead/templates/macros.pt 2009-04-01 20:11:48 +0000
74@@ -17,32 +17,23 @@
75 var expanded_icon_path = <tal:block content="python:'\''+branch.static_url('/static/images/treeExpanded.png')+'\''" />;
76 </script>
77 <script type="text/javascript"
78- tal:attributes="src
79- python:branch.static_url('/static/javascript/yui/build/yui/yui-min.js')"></script>
80- <script type="text/javascript"
81- tal:attributes="src
82- python:branch.static_url('/static/javascript/yui/build/oop/oop-min.js')"></script>
83- <script type="text/javascript"
84- tal:attributes="src
85- python:branch.static_url('/static/javascript/yui/build/event/event-min.js')"></script>
86- <script type="text/javascript"
87- tal:attributes="src
88- python:branch.static_url('/static/javascript/yui/build/attribute/attribute-min.js')"></script>
89- <script type="text/javascript"
90- tal:attributes="src
91- python:branch.static_url('/static/javascript/yui/build/base/base-min.js')"></script>
92- <script type="text/javascript"
93- tal:attributes="src
94- python:branch.static_url('/static/javascript/yui/build/dom/dom-min.js')"></script>
95- <script type="text/javascript"
96- tal:attributes="src
97- python:branch.static_url('/static/javascript/yui/build/node/node-min.js')"></script>
98- <script type="text/javascript"
99- tal:attributes="src
100- python:branch.static_url('/static/javascript/yui/build/anim/anim-min.js')"></script>
101- <script type="text/javascript"
102- tal:attributes="src
103- python:branch.static_url('/static/javascript/yui/build/io/io-base-min.js')"></script>
104+ tal:attributes="src python:branch.yui_url('yui/yui-min.js')"></script>
105+ <script type="text/javascript"
106+ tal:attributes="src python:branch.yui_url('oop/oop-min.js')"></script>
107+ <script type="text/javascript"
108+ tal:attributes="src python:branch.yui_url('event/event-min.js')"></script>
109+ <script type="text/javascript"
110+ tal:attributes="src python:branch.yui_url('attribute/attribute-min.js')"></script>
111+ <script type="text/javascript"
112+ tal:attributes="src python:branch.yui_url('base/base-min.js')"></script>
113+ <script type="text/javascript"
114+ tal:attributes="src python:branch.yui_url('dom/dom-min.js')"></script>
115+ <script type="text/javascript"
116+ tal:attributes="src python:branch.yui_url('node/node-min.js')"></script>
117+ <script type="text/javascript"
118+ tal:attributes="src python:branch.yui_url('anim/anim-min.js')"></script>
119+ <script type="text/javascript"
120+ tal:attributes="src python:branch.yui_url('io/io-base-min.js')"></script>
121 <script type="text/javascript"
122 tal:attributes="src python:branch.static_url('/static/javascript/custom.js')"></script>
123 <metal:block metal:define-slot="header_extras" />

Subscribers

People subscribed via source and target branches