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

[Martin Albisetti]

[Martin Albisetti (beuno)]

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

[Michael Hudson-Doyle (mwhudson)]

> 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.

[Martin Albisetti (beuno)]

> 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?

[Michael Hudson-Doyle (mwhudson)]

> > 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!

[lp:~beuno/loggerhead/global-config]

360. By Martin Albisetti on 2009-06-03

Add deprectation warning to start-loggerhead

=== 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 @@
+#
+# Copyright (C) 2008, 2009 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
 '''Configuration tools for Loggerhead.'''
+
 from optparse import OptionParser
 import sys
 import tempfile
 
+from bzrlib import config
 
 _temporary_sql_dir = None
 
@@ -45,9 +60,9 @@
                       type=str, help="The directory to place log files in.")
     parser.add_option("--version", action="store_true", dest="show_version",
                       help="Print the software version and exit")
-    parser.add_option('--use-cdn', action='store_true',
+    parser.add_option("--use-cdn", action="store_true", dest="use_cdn",
                       help="Serve YUI from Yahoo!'s CDN")
-    parser.add_option('--cache-dir', dest='sql_dir',
+    parser.add_option("--cache-dir", dest="sql_dir",
                       help="The directory to place the SQL cache in")
     return parser
 
@@ -67,19 +82,27 @@
         self.SQL_DIR = sql_dir
 
     def get_option(self, option):
-        '''Get an option from the options dict.'''
-        return getattr(self._options, option)
+        """Get the value for the config option, either 
+           from ~/.bazaar/bazaar.conf or from the command line.
+           All loggerhead-specific settings start with 'http_'"""
+        global_config = config.GlobalConfig().get_user_option('http_'+option)
+        cmd_config = getattr(self._options, option)
+        if global_config is not None and (
+                cmd_config is None or cmd_config is False):
+            return global_config
+        else:
+            return cmd_config
 
     def get_arg(self, index):
-        '''Get an arg from the arg list.'''
+        """Get an arg from the arg list."""
         return self._args[index]
 
     def print_help(self):
-        '''Wrapper around OptionParser.print_help.'''
+        """Wrapper around OptionParser.print_help."""
         return self._parser.print_help()
 
     @property
     def arg_count(self):
-        '''Return the number of args from the option parser.'''
+        """Return the number of args from the option parser."""
         return len(self._args)
 

=== 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 @@
 
 from bzrlib.tests import TestCaseWithTransport
 from bzrlib.util.configobj.configobj import ConfigObj
+from bzrlib import config
 
 from loggerhead.apps.branch import BranchWSGIApp
 from paste.fixture import TestApp
@@ -118,3 +119,19 @@
         res = app.get('/files')
         res.mustcontain('No revisions!')
 
+#class TestGlobalConfig(BasicTests):
+#    """
+#    Test that global config settings are respected
+#    """
+
+#    def setUp(self):
+#        BasicTests.setUp(self)
+#        self.createBranch()
+#        config.GlobalConfig().set_user_option('http_version', 'True')
+
+#    def test_setting_respected(self):
+        #FIXME: Figure out how to test this properly
+#        app = self.setUpLoggerhead()
+#        res = app.get('/changes', status=200)
+
+

=== 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 @@
 #!/usr/bin/env python
+#
+# Copyright (C) 2008, 2009 Canonical Ltd
+#
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
 # the Free Software Foundation; either version 2 of the License, or