loggerhead

Merge lp://staging/~j/loggerhead/loggerhead-add_url-prefix_to_serve-branches into lp://staging/loggerhead

  1. loggerhead-add_url-prefix_to_serve-branches
  2. Merge into trunk-rich
Proposed by j^
Status: Work in progress
Proposed branch: lp://staging/~j/loggerhead/loggerhead-add_url-prefix_to_serve-branches
Merge into: lp://staging/loggerhead
Diff against target: 57 lines (+11/-0)
3 files modified
loggerhead/apps/transport.py (+5/-0)
loggerhead/config.py (+3/-0)
serve-branches.1 (+3/-0)
To merge this branch: bzr merge lp://staging/~j/loggerhead/loggerhead-add_url-prefix_to_serve-branches
Reviewer Review Type Date Requested Status
Matt Nordhoff Needs Fixing
Review via email: mp+20575@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
j^ (j) wrote :

since all branches served via serve-branches are available, i want to advertise them,
this patch adds an option --url-prefix that works like url_prefix in loggerhead.conf

Reply
lp://staging/~j/loggerhead/loggerhead-add_url-prefix_to_serve-branches updated
403. By j^

use object() as default

404. By j^

make that None

Revision history for this message
Martin Pool (mbp) wrote :

This seems reasonable to me, but I'd like to be more clear on just what the impact is. Can we add an example to the manual perhaps?

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

A couple of quick comments:

9 + served_url = self._config.get_option('url_prefix') + name

I think it would be nice to run this through paste.request.resolve_relative_url() somewhere along the line.

11 + served_url = None

Changing served_url's default from loggerhead.apps.branch._DEFAULT to None will break things.

40 + help="All branches will be advertised to be available from this prefix")

This line's too long -- needs to be wrapped.

45 === modified file 'serve-branches.1'

Wow, you remembered the man page. I had forgotten it existed. \o/

review: Needs Fixing
Reply
Revision history for this message
Kevin Jadin (marcspitz) wrote :

I have been able to adapt this patch to loggerhead 1.18.
I am not that good in merging and all that stuff.

Can someone more experienced than I am please follow the request that Matt Nordhoff made in order to merge the diff into loggerhead's trunk?

It shouldn't take that long to edit 4 lines of code just to enjoy this feature..
Thanks

PS : for this to work, I also had to add:
--url-prefix=$url_prefix in /etc/init.d/loggerhead
url_preview=http://mysite/url
to make this work, maybe this should also be edited in sources, but I don't know where..

Reply
Revision history for this message
Kevin Jadin (marcspitz) wrote :

of course: url_preview=http://mysite/url in /etc/serve-branches.conf

Reply

Unmerged revisions

404. By j^

make that None

403. By j^

use object() as default

402. By j^

add option to advertice all branches served by serve-branches to be available from given prefix (inline with loggeread.conf url_prefix option)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'loggerhead/apps/transport.py'
2--- loggerhead/apps/transport.py 2009-11-26 03:20:53 +0000
3+++ loggerhead/apps/transport.py 2010-03-03 17:33:15 +0000
4@@ -54,6 +54,10 @@
5 else:
6 name = self.name
7 is_root = False
8+ if self._config.get_option('url_prefix'):
9+ served_url = self._config.get_option('url_prefix') + name
10+ else:
11+ served_url = None
12 branch_app = BranchWSGIApp(
13 branch,
14 name,
15@@ -61,6 +65,7 @@
16 self.root.graph_cache,
17 is_root=is_root,
18 use_cdn=self._config.get_option('use_cdn'),
19+ served_url=served_url
20 )
21 return branch_app.app
22
23
24=== modified file 'loggerhead/config.py'
25--- loggerhead/config.py 2009-11-23 22:45:46 +0000
26+++ loggerhead/config.py 2010-03-03 17:33:15 +0000
27@@ -36,6 +36,7 @@
28 use_cdn=False,
29 sql_dir=None,
30 allow_writes=False,
31+ url_prefix=None,
32 )
33 parser.add_option("--user-dirs", action="store_true",
34 help="Serve user directories as ~user.")
35@@ -69,6 +70,8 @@
36 help="The directory to place the SQL cache in")
37 parser.add_option("--allow-writes", action="store_true",
38 help="Allow writing to the Bazaar server.")
39+ parser.add_option('--url-prefix', dest='url_prefix',
40+ help="All branches will be advertised to be available from this prefix")
41 return parser
42
43
44
45=== modified file 'serve-branches.1'
46--- serve-branches.1 2008-09-10 23:12:59 +0000
47+++ serve-branches.1 2010-03-03 17:33:15 +0000
48@@ -38,6 +38,9 @@
49 .TP
50 \fB\-\-version\fR
51 Print the software version and exit.
52+.TP
53+\fB\-\-url-prefix\fR
54+All branches will be advertised to be available from this prefix.
55 .SH "LICENSE"
56 GNU GPLv2 or later.
57 .SH "SEE ALSO"

Subscribers

People subscribed via source and target branches