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
=== modified file 'loggerhead/config.py'
--- loggerhead/config.py 2009-05-18 05:16:38 +0000
+++ loggerhead/config.py 2009-05-29 13:25:59 +0000
@@ -1,8 +1,23 @@
1#
2# Copyright (C) 2008, 2009 Canonical Ltd
3#
4# This program is free software; you can redistribute it and/or modify
5# it under the terms of the GNU General Public License as published by
6# the Free Software Foundation; either version 2 of the License, or
7# (at your option) any later version.
8#
9# This program is distributed in the hope that it will be useful,
10# but WITHOUT ANY WARRANTY; without even the implied warranty of
11# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12# GNU General Public License for more details.
13#
1'''Configuration tools for Loggerhead.'''14'''Configuration tools for Loggerhead.'''
15
2from optparse import OptionParser16from optparse import OptionParser
3import sys17import sys
4import tempfile18import tempfile
519
20from bzrlib import config
621
7_temporary_sql_dir = None22_temporary_sql_dir = None
823
@@ -45,9 +60,9 @@
45 type=str, help="The directory to place log files in.")60 type=str, help="The directory to place log files in.")
46 parser.add_option("--version", action="store_true", dest="show_version",61 parser.add_option("--version", action="store_true", dest="show_version",
47 help="Print the software version and exit")62 help="Print the software version and exit")
48 parser.add_option('--use-cdn', action='store_true',63 parser.add_option("--use-cdn", action="store_true", dest="use_cdn",
49 help="Serve YUI from Yahoo!'s CDN")64 help="Serve YUI from Yahoo!'s CDN")
50 parser.add_option('--cache-dir', dest='sql_dir',65 parser.add_option("--cache-dir", dest="sql_dir",
51 help="The directory to place the SQL cache in")66 help="The directory to place the SQL cache in")
52 return parser67 return parser
5368
@@ -67,19 +82,27 @@
67 self.SQL_DIR = sql_dir82 self.SQL_DIR = sql_dir
6883
69 def get_option(self, option):84 def get_option(self, option):
70 '''Get an option from the options dict.'''85 """Get the value for the config option, either
71 return getattr(self._options, option)86 from ~/.bazaar/bazaar.conf or from the command line.
87 All loggerhead-specific settings start with 'http_'"""
88 global_config = config.GlobalConfig().get_user_option('http_'+option)
89 cmd_config = getattr(self._options, option)
90 if global_config is not None and (
91 cmd_config is None or cmd_config is False):
92 return global_config
93 else:
94 return cmd_config
7295
73 def get_arg(self, index):96 def get_arg(self, index):
74 '''Get an arg from the arg list.'''97 """Get an arg from the arg list."""
75 return self._args[index]98 return self._args[index]
7699
77 def print_help(self):100 def print_help(self):
78 '''Wrapper around OptionParser.print_help.'''101 """Wrapper around OptionParser.print_help."""
79 return self._parser.print_help()102 return self._parser.print_help()
80103
81 @property104 @property
82 def arg_count(self):105 def arg_count(self):
83 '''Return the number of args from the option parser.'''106 """Return the number of args from the option parser."""
84 return len(self._args)107 return len(self._args)
85108
86109
=== modified file 'loggerhead/tests/test_simple.py'
--- loggerhead/tests/test_simple.py 2009-02-18 10:40:22 +0000
+++ loggerhead/tests/test_simple.py 2009-05-29 13:25:59 +0000
@@ -3,6 +3,7 @@
33
4from bzrlib.tests import TestCaseWithTransport4from bzrlib.tests import TestCaseWithTransport
5from bzrlib.util.configobj.configobj import ConfigObj5from bzrlib.util.configobj.configobj import ConfigObj
6from bzrlib import config
67
7from loggerhead.apps.branch import BranchWSGIApp8from loggerhead.apps.branch import BranchWSGIApp
8from paste.fixture import TestApp9from paste.fixture import TestApp
@@ -118,3 +119,19 @@
118 res = app.get('/files')119 res = app.get('/files')
119 res.mustcontain('No revisions!')120 res.mustcontain('No revisions!')
120121
122#class TestGlobalConfig(BasicTests):
123# """
124# Test that global config settings are respected
125# """
126
127# def setUp(self):
128# BasicTests.setUp(self)
129# self.createBranch()
130# config.GlobalConfig().set_user_option('http_version', 'True')
131
132# def test_setting_respected(self):
133 #FIXME: Figure out how to test this properly
134# app = self.setUpLoggerhead()
135# res = app.get('/changes', status=200)
136
137
121138
=== modified file 'serve-branches'
--- serve-branches 2009-05-04 20:21:08 +0000
+++ serve-branches 2009-05-28 10:38:06 +0000
@@ -1,4 +1,7 @@
1#!/usr/bin/env python1#!/usr/bin/env python
2#
3# Copyright (C) 2008, 2009 Canonical Ltd
4#
2# This program is free software; you can redistribute it and/or modify5# This program is free software; you can redistribute it and/or modify
3# it under the terms of the GNU General Public License as published by6# it under the terms of the GNU General Public License as published by
4# the Free Software Foundation; either version 2 of the License, or7# the Free Software Foundation; either version 2 of the License, or

Subscribers

People subscribed via source and target branches