Rev 5779: Merge config-concrete-stores into config-section-matchers in file:///home/vila/src/bzr/experimental/config/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Tue May 3 09:23:55 UTC 2011
At file:///home/vila/src/bzr/experimental/config/
------------------------------------------------------------
revno: 5779 [merge]
revision-id: v.ladeuil+lp at free.fr-20110503092355-gf6680de3g7607m1
parent: v.ladeuil+lp at free.fr-20110502080453-jv0vmxg0p34ggj24
parent: v.ladeuil+lp at free.fr-20110503092215-50y0xylk73fimj6h
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: config-section-matchers
timestamp: Tue 2011-05-03 11:23:55 +0200
message:
Merge config-concrete-stores into config-section-matchers
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:22:50 +0000
+++ b/bzrlib/config.py 2011-05-03 09:23:55 +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,18 @@
@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):
+
+# FIXME: Moreover, we shouldn't need classes for these stores either, factory
+# functions or a registry will make it easier and clearer for tests, focusing
+# on the relevant parts of the API that needs testing -- vila 20110503 (based
+# on a poolie's remark)
+class GlobalStore(LockableIniFileStore):
def __init__(self, possible_transports=None):
t = transport.get_transport(config_dir(),
@@ -2322,7 +2350,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 +2358,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:53 +0000
+++ b/bzrlib/tests/test_config.py 2011-05-03 09:23:55 +0000
@@ -63,15 +63,21 @@
load_tests = scenarios.load_tests_apply_scenarios
-# We need adpaters that can build a config store in a test context. Test
+# We need adapters that can build a config store in a test context. Test
# classes, based on TestCaseWithTransport, can use the registry to parametrize
# themselves. The builder will receive a test instance and should return a
# ready-to-use store. Plugins that defines new stores can also register
# themselves here to be tested against the tests defined below.
+
+# FIXME: plugins should *not* need to import test_config to register their
+# helpers (or selftest -s xxx will be broken), the following registry should be
+# moved to bzrlib.config instead so that selftest -s bt.test_config also runs
+# the plugin specific tests (selftest -s bp.xxx won't, that would be against
+# the spirit of '-s') -- vila 20110503
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 +1838,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 +1932,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 +2042,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 +2092,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 16:13:23 +0000
+++ b/doc/developers/configuration.txt 2011-05-03 09:23:55 +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