Merge lp://staging/~beuno/loggerhead/global-config into lp://staging/loggerhead

Proposed by Martin Albisetti
Status: Merged
Merged at revision: not available
Proposed branch: lp://staging/~beuno/loggerhead/global-config
Merge into: lp://staging/loggerhead
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~beuno/loggerhead/global-config
Reviewer Review Type Date Requested Status
Michael Hudson-Doyle Pending
Loggerhead Team Pending
Review via email: mp+6931@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Martin Albisetti (beuno) wrote :

This branch allows setting configs in ~/.bazaar/bazaar.conf, which are overridden by the command line.
I think this branch has 3 things to consider:

1. Testing. I can't figure out how to test it, as all options seem to excersized by serve-branches, which isn't invoked in tests

2. Add a big fat deprecation warning to start-loggerhead script

3. I'm not 100% sure how to (properly) detect when something is set on the command line, so I hacked my way around it.

Any help is very much appreciated

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

> This branch allows setting configs in ~/.bazaar/bazaar.conf, which are
> overridden by the command line.
> I think this branch has 3 things to consider:
>
> 1. Testing. I can't figure out how to test it, as all options seem to
> excersized by serve-branches, which isn't invoked in tests

Yeah, there's far too much code in serve-branches. I guess a lot of it could live in loggerhead/config.py.

> 2. Add a big fat deprecation warning to start-loggerhead script

+1

> 3. I'm not 100% sure how to (properly) detect when something is set on the
> command line, so I hacked my way around it.

We can probably do this with a custom optparse action. Maybe I'll dig into this when I'm more awake.

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

> Yeah, there's far too much code in serve-branches. I guess a lot of it could
> live in loggerhead/config.py.

Do you think that's a blocker for this branch to land?

> > 2. Add a big fat deprecation warning to start-loggerhead script
>
> +1

done.

> > 3. I'm not 100% sure how to (properly) detect when something is set on the
> > command line, so I hacked my way around it.
>
> We can probably do this with a custom optparse action. Maybe I'll dig into
> this when I'm more awake.

Can that be done post-landing?

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

> > Yeah, there's far too much code in serve-branches. I guess a lot of it
> could
> > live in loggerhead/config.py.
>
> Do you think that's a blocker for this branch to land?

No, I guess not.

> > > 2. Add a big fat deprecation warning to start-loggerhead script
> >
> > +1
>
> done.
>

Cool.

> > > 3. I'm not 100% sure how to (properly) detect when something is set on the
> > > command line, so I hacked my way around it.
> >
> > We can probably do this with a custom optparse action. Maybe I'll dig into
> > this when I'm more awake.
>
> Can that be done post-landing?

I guess.

Land it, I guess I'm saying!

360. By Martin Albisetti

Add deprectation warning to start-loggerhead

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'loggerhead/config.py'
2--- loggerhead/config.py 2009-05-18 05:16:38 +0000
3+++ loggerhead/config.py 2009-05-29 13:25:59 +0000
4@@ -1,8 +1,23 @@
5+#
6+# Copyright (C) 2008, 2009 Canonical Ltd
7+#
8+# This program is free software; you can redistribute it and/or modify
9+# it under the terms of the GNU General Public License as published by
10+# the Free Software Foundation; either version 2 of the License, or
11+# (at your option) any later version.
12+#
13+# This program is distributed in the hope that it will be useful,
14+# but WITHOUT ANY WARRANTY; without even the implied warranty of
15+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16+# GNU General Public License for more details.
17+#
18 '''Configuration tools for Loggerhead.'''
19+
20 from optparse import OptionParser
21 import sys
22 import tempfile
23
24+from bzrlib import config
25
26 _temporary_sql_dir = None
27
28@@ -45,9 +60,9 @@
29 type=str, help="The directory to place log files in.")
30 parser.add_option("--version", action="store_true", dest="show_version",
31 help="Print the software version and exit")
32- parser.add_option('--use-cdn', action='store_true',
33+ parser.add_option("--use-cdn", action="store_true", dest="use_cdn",
34 help="Serve YUI from Yahoo!'s CDN")
35- parser.add_option('--cache-dir', dest='sql_dir',
36+ parser.add_option("--cache-dir", dest="sql_dir",
37 help="The directory to place the SQL cache in")
38 return parser
39
40@@ -67,19 +82,27 @@
41 self.SQL_DIR = sql_dir
42
43 def get_option(self, option):
44- '''Get an option from the options dict.'''
45- return getattr(self._options, option)
46+ """Get the value for the config option, either
47+ from ~/.bazaar/bazaar.conf or from the command line.
48+ All loggerhead-specific settings start with 'http_'"""
49+ global_config = config.GlobalConfig().get_user_option('http_'+option)
50+ cmd_config = getattr(self._options, option)
51+ if global_config is not None and (
52+ cmd_config is None or cmd_config is False):
53+ return global_config
54+ else:
55+ return cmd_config
56
57 def get_arg(self, index):
58- '''Get an arg from the arg list.'''
59+ """Get an arg from the arg list."""
60 return self._args[index]
61
62 def print_help(self):
63- '''Wrapper around OptionParser.print_help.'''
64+ """Wrapper around OptionParser.print_help."""
65 return self._parser.print_help()
66
67 @property
68 def arg_count(self):
69- '''Return the number of args from the option parser.'''
70+ """Return the number of args from the option parser."""
71 return len(self._args)
72
73
74=== modified file 'loggerhead/tests/test_simple.py'
75--- loggerhead/tests/test_simple.py 2009-02-18 10:40:22 +0000
76+++ loggerhead/tests/test_simple.py 2009-05-29 13:25:59 +0000
77@@ -3,6 +3,7 @@
78
79 from bzrlib.tests import TestCaseWithTransport
80 from bzrlib.util.configobj.configobj import ConfigObj
81+from bzrlib import config
82
83 from loggerhead.apps.branch import BranchWSGIApp
84 from paste.fixture import TestApp
85@@ -118,3 +119,19 @@
86 res = app.get('/files')
87 res.mustcontain('No revisions!')
88
89+#class TestGlobalConfig(BasicTests):
90+# """
91+# Test that global config settings are respected
92+# """
93+
94+# def setUp(self):
95+# BasicTests.setUp(self)
96+# self.createBranch()
97+# config.GlobalConfig().set_user_option('http_version', 'True')
98+
99+# def test_setting_respected(self):
100+ #FIXME: Figure out how to test this properly
101+# app = self.setUpLoggerhead()
102+# res = app.get('/changes', status=200)
103+
104+
105
106=== modified file 'serve-branches'
107--- serve-branches 2009-05-04 20:21:08 +0000
108+++ serve-branches 2009-05-28 10:38:06 +0000
109@@ -1,4 +1,7 @@
110 #!/usr/bin/env python
111+#
112+# Copyright (C) 2008, 2009 Canonical Ltd
113+#
114 # This program is free software; you can redistribute it and/or modify
115 # it under the terms of the GNU General Public License as published by
116 # the Free Software Foundation; either version 2 of the License, or

Subscribers

People subscribed via source and target branches