Rev 5776: Merge config-abstract-store into config-concrete-stores resolving conflicts in file:///home/vila/src/bzr/experimental/config/

Vincent Ladeuil v.ladeuil+lp at free.fr
Tue May 3 08:47:38 UTC 2011


At file:///home/vila/src/bzr/experimental/config/

------------------------------------------------------------
revno: 5776 [merge]
revision-id: v.ladeuil+lp at free.fr-20110503084737-jj07y2ujws43t520
parent: v.ladeuil+lp at free.fr-20110502080451-h2i1y7lyzxs4k30e
parent: v.ladeuil+lp at free.fr-20110503084230-5y1o5f116rtsp6a0
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: config-concrete-stores
timestamp: Tue 2011-05-03 10:47:37 +0200
message:
  Merge config-abstract-store into config-concrete-stores resolving conflicts
modified:
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
  doc/developers/configuration.txt configuration.txt-20110408142435-korjxxnskvq44sta-1
  doc/en/user-guide/configuring_bazaar.txt configuring_bazaar.t-20071128000722-ncxiua259xwbdbg7-1
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-04-12 11:21:07 +0000
+++ b/bzrlib/config.py	2011-05-03 08:47:37 +0000
@@ -2094,11 +2094,12 @@
         self._transport.put_file(self._filename, out_file)
 
 
-class ReadOnlySection(object):
+class Section(object):
     """A section defines a dict of options.
 
     This is merely a read-only dict which can add some knowledge about the
-    options.
+    options. It is *not* a python dict object though and doesn't try to mimic
+    its API.
     """
 
     def __init__(self, section_id, options):
@@ -2109,12 +2110,16 @@
     def get(self, name, default=None):
         return self.options.get(name, default)
 
+    def __repr__(self):
+        # Mostly for debugging use
+        return "<config.%s id=%s>" % (self.__class__.__name__, self.id)
+
 
 _NewlyCreatedOption = object()
 """Was the option created during the MutableSection lifetime"""
 
 
-class MutableSection(ReadOnlySection):
+class MutableSection(Section):
     """A section allowing changes and keeping track of the original values."""
 
     def __init__(self, section_id, options):
@@ -2138,14 +2143,19 @@
 class Store(object):
     """Abstract interface to persistent storage for configuration options."""
 
-    readonly_section_class = ReadOnlySection
+    readonly_section_class = Section
     mutable_section_class = MutableSection
 
-    @property
-    def loaded(self):
-        raise NotImplementedError(self.loaded)
+    def is_loaded(self):
+        """Returns True if the Store has been loaded.
+
+        This is used to implement lazy loading and ensure the persistent
+        storage is queried only when needed.
+        """
+        raise NotImplementedError(self.is_loaded)
 
     def load(self):
+        """Loads the Store from persistent storage."""
         raise NotImplementedError(self.load)
 
     def _load_from_string(self, str_or_unicode):
@@ -2160,6 +2170,7 @@
         raise NotImplementedError(self._load_from_string)
 
     def save(self):
+        """Saves the Store to persistent storage."""
         raise NotImplementedError(self.save)
 
     def external_url(self):
@@ -2179,8 +2190,21 @@
         """
         raise NotImplementedError(self.get_mutable_section)
 
-
-class ConfigObjStore(Store):
+    def __repr__(self):
+        # Mostly for debugging use
+        return "<config.%s id=%s>" % (self.__class__.__name__, self.id)
+
+
+class IniFileStore(Store):
+    """A config Store using ConfigObj for storage.
+
+    :ivar transport: The transport object where the config file is located.
+
+    :ivar file_name: The config file basename in the transport directory.
+
+    :ivar _config_obj: Private member to hold the ConfigObj instance used to
+        serialize/deserialize the config file.
+    """
 
     def __init__(self, transport, file_name):
         """A config Store using ConfigObj for storage.
@@ -2189,18 +2213,17 @@
 
         :param file_name: The config file basename in the transport directory.
         """
-        super(ConfigObjStore, self).__init__()
+        super(IniFileStore, self).__init__()
         self.transport = transport
         self.file_name = file_name
         self._config_obj = None
 
-    @property
-    def loaded(self):
+    def is_loaded(self):
         return self._config_obj != None
 
     def load(self):
         """Load the store from the associated file."""
-        if self.loaded:
+        if self.is_loaded():
             return
         content = self.transport.get_bytes(self.file_name)
         self._load_from_string(content)
@@ -2214,7 +2237,7 @@
         This is for tests and should not be used in production unless a
         convincing use case can be demonstrated :)
         """
-        if self.loaded:
+        if self.is_loaded():
             raise AssertionError('Already loaded: %r' % (self._config_obj,))
         co_input = StringIO(str_or_unicode.encode('utf-8'))
         try:
@@ -2225,7 +2248,7 @@
             raise errors.ParseConfigError(e.errors, self.external_url())
 
     def save(self):
-        if not self.loaded:
+        if not self.is_loaded():
             # Nothing to save
             return
         out = StringIO()
@@ -2238,7 +2261,7 @@
         # The following will do in the interim but maybe we don't want to
         # expose a path here but rather a config ID and its associated
         # object </hand wawe>.
-        return os.path.join(self.transport.external_url(), self.file_name)
+        return urlutils.join(self.transport.external_url(), self.file_name)
 
     def get_sections(self):
         """Get the configobj section in the file order.
@@ -2273,7 +2296,7 @@
 # they may face the same issue.
 
 
-class LockableConfigObjStore(ConfigObjStore):
+class LockableIniFileStore(IniFileStore):
     """A ConfigObjStore using locks on save to ensure store integrity."""
 
     def __init__(self, transport, file_name, lock_dir_name=None):
@@ -2286,7 +2309,7 @@
         if lock_dir_name is None:
             lock_dir_name = 'lock'
         self.lock_dir_name = lock_dir_name
-        super(LockableConfigObjStore, self).__init__(transport, file_name)
+        super(LockableIniFileStore, self).__init__(transport, file_name)
         self._lock = lockdir.LockDir(self.transport, self.lock_dir_name)
 
     def lock_write(self, token=None):
@@ -2308,13 +2331,13 @@
 
     @needs_write_lock
     def save(self):
-        super(LockableConfigObjStore, self).save()
+        super(LockableIniFileStore, self).save()
 
 
 # FIXME: global, bazaar, shouldn't that be 'user' instead or even
 # 'user_defaults' as opposed to 'user_overrides', 'system_defaults'
 # (/etc/bzr/bazaar.conf) and 'system_overrides' ? -- vila 2011-04-05
-class GlobalStore(LockableConfigObjStore):
+class GlobalStore(LockableIniFileStore):
 
     def __init__(self, possible_transports=None):
         t = transport.get_transport(config_dir(),
@@ -2322,7 +2345,7 @@
         super(GlobalStore, self).__init__(t, 'bazaar.conf')
 
 
-class LocationStore(LockableConfigObjStore):
+class LocationStore(LockableIniFileStore):
 
     def __init__(self, possible_transports=None):
         t = transport.get_transport(config_dir(),
@@ -2330,7 +2353,7 @@
         super(LocationStore, self).__init__(t, 'locations.conf')
 
 
-class BranchStore(ConfigObjStore):
+class BranchStore(IniFileStore):
 
     def __init__(self, branch):
         super(BranchStore, self).__init__(branch.control_transport,

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2011-05-02 08:04:51 +0000
+++ b/bzrlib/tests/test_config.py	2011-05-03 08:47:37 +0000
@@ -70,8 +70,8 @@
 # themselves here to be tested against the tests defined below.
 test_store_builder_registry = registry.Registry()
 test_store_builder_registry.register(
-    'configobj', lambda test: config.ConfigObjStore(test.get_transport(),
-                                                    'configobj.conf'))
+    'configobj', lambda test: config.IniFileStore(test.get_transport(),
+                                                  'configobj.conf'))
 test_store_builder_registry.register(
     'bazaar', lambda test: config.GlobalStore())
 test_store_builder_registry.register(
@@ -1832,29 +1832,29 @@
         self.assertIs(None, bzrdir_config.get_default_stack_on())
 
 
-class TestConfigReadOnlySection(tests.TestCase):
+class TestSection(tests.TestCase):
 
     # FIXME: Parametrize so that all sections produced by Stores run these
     # tests -- vila 2011-04-01
 
     def test_get_a_value(self):
         a_dict = dict(foo='bar')
-        section = config.ReadOnlySection('myID', a_dict)
+        section = config.Section('myID', a_dict)
         self.assertEquals('bar', section.get('foo'))
 
-    def test_get_unkown_option(self):
+    def test_get_unknown_option(self):
         a_dict = dict()
-        section = config.ReadOnlySection(None, a_dict)
+        section = config.Section(None, a_dict)
         self.assertEquals('out of thin air',
                           section.get('foo', 'out of thin air'))
 
     def test_options_is_shared(self):
         a_dict = dict()
-        section = config.ReadOnlySection(None, a_dict)
+        section = config.Section(None, a_dict)
         self.assertIs(a_dict, section.options)
 
 
-class TestConfigMutableSection(tests.TestCase):
+class TestMutableSection(tests.TestCase):
 
     # FIXME: Parametrize so that all sections (including os.environ and the
     # ones produced by Stores) run these tests -- vila 2011-04-01
@@ -1926,9 +1926,9 @@
 
     def test_building_delays_load(self):
         store = self.get_store(self)
-        self.assertEquals(False, store.loaded)
+        self.assertEquals(False, store.is_loaded())
         store._load_from_string('')
-        self.assertEquals(True, store.loaded)
+        self.assertEquals(True, store.is_loaded())
 
     def test_get_no_sections_for_empty(self):
         store = self.get_store(self)
@@ -2036,28 +2036,28 @@
         self.assertSectionContent(('baz', {'foo': 'bar'}), sections[0])
 
 
-class TestConfigObjStore(TestStore):
+class TestIniFileStore(TestStore):
 
     def test_loading_unknown_file_fails(self):
-        store = config.ConfigObjStore(self.get_transport(), 'I-do-not-exist')
+        store = config.IniFileStore(self.get_transport(), 'I-do-not-exist')
         self.assertRaises(errors.NoSuchFile, store.load)
 
     def test_invalid_content(self):
-        store = config.ConfigObjStore(self.get_transport(), 'foo.conf', )
-        self.assertEquals(False, store.loaded)
+        store = config.IniFileStore(self.get_transport(), 'foo.conf', )
+        self.assertEquals(False, store.is_loaded())
         exc = self.assertRaises(
             errors.ParseConfigError, store._load_from_string,
             'this is invalid !')
         self.assertEndsWith(exc.filename, 'foo.conf')
         # And the load failed
-        self.assertEquals(False, store.loaded)
+        self.assertEquals(False, store.is_loaded())
 
     def test_get_embedded_sections(self):
         # A more complicated example (which also shows that section names and
         # option names share the same name space...)
         # FIXME: This should be fixed by forbidding dicts as values ?
         # -- vila 2011-04-05
-        store = config.ConfigObjStore(self.get_transport(), 'foo.conf', )
+        store = config.IniFileStore(self.get_transport(), 'foo.conf', )
         store._load_from_string('''
 foo=bar
 l=1,2
@@ -2086,17 +2086,17 @@
             sections[3])
 
 
-class TestLockableConfigObjStore(TestStore):
+class TestLockableIniFileStore(TestStore):
 
     def test_create_store_in_created_dir(self):
         t = self.get_transport('dir/subdir')
-        store = config.LockableConfigObjStore(t, 'foo.conf')
+        store = config.LockableIniFileStore(t, 'foo.conf')
         store.get_mutable_section(None).set('foo', 'bar')
         store.save()
 
     # FIXME: We should adapt the tests in TestLockableConfig about concurrent
     # writes. Since this requires a clearer rewrite, I'll just rely on using
-    # the same code in LockableConfigObjStore (copied from LockableConfig, but
+    # the same code in LockableIniFileStore (copied from LockableConfig, but
     # trivial enough, the main difference is that we add @needs_write_lock on
     # save() instead of set_user_option() and remove_user_option()). The intent
     # is to ensure that we always get a valid content for the store even when

=== modified file 'doc/developers/configuration.txt'
--- a/doc/developers/configuration.txt	2011-04-08 15:59:11 +0000
+++ b/doc/developers/configuration.txt	2011-05-03 08:47:37 +0000
@@ -18,21 +18,21 @@
 - you can get, set and remove an option,
 - the value is a unicode string.
 
-MutableSection are needed to set or remove an option, ReadOnlySection should
+MutableSection is needed to set or remove an option, ReadOnlySection should
 be used otherwise.
 
 Stores
 ------
 
-Options can persistent in which case they are saved into Stores.
+Options can be persistent in which case they are saved into Stores.
 
 ``config.Store`` defines the abstract interface that all stores should
 implement.
 
-This object doesn't provide a direct access to the options, it only provides
-access to Sections. This is deliberate to ensure that sections can be properly
-shared by reusing the same underlying objects. Accessing options should be
-done via the ``Section`` objects.
+This object doesn't provide direct access to the options, it only provides
+access to Sections. This is deliberate to ensure that sections can be
+properly shared by reusing the same underlying objects. Accessing options
+should be done via the ``Section`` objects.
 
 A ``Store`` can contain one or more sections, each section is uniquely
 identified by a unicode string.

=== modified file 'doc/en/user-guide/configuring_bazaar.txt'
--- a/doc/en/user-guide/configuring_bazaar.txt	2011-04-08 14:59:29 +0000
+++ b/doc/en/user-guide/configuring_bazaar.txt	2011-05-02 08:15:59 +0000
@@ -41,19 +41,19 @@
 -------------------------
 
 As shown in the example above, there are various ways to
-configure Bazaar, they all share some common properties though,
-an option has:
+configure Bazaar, they all share some common properties though.
+An option has:
 
 - a name which is generally a valid python identifier,
 
-- a value which is a string. In some cases, Bazzar will be able
+- a value which is a string. In some cases, Bazaar will be able
   to recognize special values like 'True', 'False' to infer a
   boolean type, but basically, as a user, you will always specify
   a value as a string.
 
-Options are grouped in various contexts so their name uniquely
-identify them in this context. When needed, options can be made
-persistent by recording them in a configuration file.
+Options are grouped in various contexts so the option name
+uniquely identifies it in this context. When needed, options can
+be made persistent by recording them in a configuration file.
 
 
 Configuration files



More information about the bazaar-commits mailing list