Rev 4234: (robertc) Reinstate the use of the Branch.get_config_file smart verb. in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu Apr 2 07:19:50 BST 2009


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 4234
revision-id: pqm at pqm.ubuntu.com-20090402061945-oh2qwvhwlpbeyyx4
parent: pqm at pqm.ubuntu.com-20090402053229-w8oy2wc2dpahfph9
parent: robertc at robertcollins.net-20090402050517-4m3m3fq2ttfvwoi0
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2009-04-02 07:19:45 +0100
message:
  (robertc) Reinstate the use of the Branch.get_config_file smart verb.
  	(Robert Collins, Andrew Bennetts)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/branch.py               branch.py-20050309040759-e4baf4e0d046576e
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
  bzrlib/smart/branch.py         branch.py-20061124031907-mzh3pla28r83r97f-1
  bzrlib/tests/blackbox/test_branch.py test_branch.py-20060524161337-noms9gmcwqqrfi8y-1
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
  bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
    ------------------------------------------------------------
    revno: 4226.1.7
    revision-id: robertc at robertcollins.net-20090402050517-4m3m3fq2ttfvwoi0
    parent: robertc at robertcollins.net-20090402040123-x690lf0614rqgypb
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: integration
    timestamp: Thu 2009-04-02 16:05:17 +1100
    message:
      Alter test_config.FakeBranch in accordance with the Branch change to have a _get_config.
    modified:
      bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
    ------------------------------------------------------------
    revno: 4226.1.6
    revision-id: robertc at robertcollins.net-20090402040123-x690lf0614rqgypb
    parent: robertc at robertcollins.net-20090402033730-i0qrdiqnldwqke02
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: integration
    timestamp: Thu 2009-04-02 15:01:23 +1100
    message:
      Revert an overly optimistic change to the Remote acceptance tests - they are not improved yet.
    modified:
      bzrlib/tests/blackbox/test_branch.py test_branch.py-20060524161337-noms9gmcwqqrfi8y-1
      bzrlib/tests/blackbox/test_push.py test_push.py-20060329002750-929af230d5d22663
    ------------------------------------------------------------
    revno: 4226.1.5
    revision-id: robertc at robertcollins.net-20090402033730-i0qrdiqnldwqke02
    parent: robertc at robertcollins.net-20090401054558-i6u5qxltq0q096or
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: RemoteBranchConfig
    timestamp: Thu 2009-04-02 14:37:30 +1100
    message:
      Reinstate the use of the Branch.get_config_file verb.
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/branch.py               branch.py-20050309040759-e4baf4e0d046576e
      bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
      bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
      bzrlib/smart/branch.py         branch.py-20061124031907-mzh3pla28r83r97f-1
      bzrlib/tests/blackbox/test_branch.py test_branch.py-20060524161337-noms9gmcwqqrfi8y-1
      bzrlib/tests/blackbox/test_push.py test_push.py-20060329002750-929af230d5d22663
      bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
=== modified file 'NEWS'
--- a/NEWS	2009-04-02 04:12:11 +0000
+++ b/NEWS	2009-04-02 06:19:45 +0000
@@ -12,6 +12,12 @@
 Compatibility Breaks
 ********************
 
+* A previously disabled code path to accelerate getting configuration
+  settings from a smart server has been reinstated. We think this *may*
+  cause a incompatibility with servers older than bzr 0.15. We intend
+  to issue a point release to address this if it turns out to be a
+  problem. (Robert Collins, Andrew Bennetts)
+
 * bzr no longer autodetects nested trees as 'tree-references'.  They
   must now be explicitly added tree references.  At the commandline, use
   join --reference instead of add.  (Aaron Bentley)
@@ -233,6 +239,11 @@
 Internals
 *********
 
+* ``Branch._get_config`` has been added, which splits out access to the
+  specific config file from the branch. This is used to let RemoteBranch
+  avoid constructing real branch objects to access configuration settings.
+  (Robert Collins, Andrew Bennetts)
+
 * ``Branch`` now implements ``set_stacked_on_url`` in the base class as
   the implementation is generic and should impact foreign formats. This
   helps performance for ``RemoteBranch`` push operations to new stacked
@@ -275,6 +286,11 @@
   handle existing svn properties that define a list of keywords to be
   expanded.  (Ian Clatworthy)
 
+* ``RemoteBranch`` now provides ``_get_config`` for access to just the
+  branch specific configuration from a remote server, which uses the 
+  already existing ``Branch.get_config_file`` smart verb.
+  (Robert Collins, Andrew Bennetts)
+
 * ``RemoteRepository`` will now negatively cache missing revisions during
   ``get_parent_map`` while read-locked. Write-locks are unaffected.
   (Robert Collins, Andrew Bennetts)

=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py	2009-04-01 05:45:58 +0000
+++ b/bzrlib/branch.py	2009-04-02 03:37:30 +0000
@@ -36,7 +36,7 @@
         ui,
         urlutils,
         )
-from bzrlib.config import BranchConfig
+from bzrlib.config import BranchConfig, TransportConfig
 from bzrlib.repofmt.pack_repo import RepositoryFormatKnitPack5RichRoot
 from bzrlib.tag import (
     BasicTags,
@@ -167,6 +167,18 @@
     def get_config(self):
         return BranchConfig(self)
 
+    def _get_config(self):
+        """Get the concrete config for just the config in this branch.
+
+        This is not intended for client use; see Branch.get_config for the
+        public API.
+
+        Added in 1.14.
+
+        :return: An object supporting get_option and set_option.
+        """
+        raise NotImplementedError(self._get_config)
+
     def _get_fallback_repository(self, url):
         """Get the repository we fallback to at url."""
         url = urlutils.join(self.base, url)
@@ -1880,6 +1892,9 @@
 
     base = property(_get_base, doc="The URL for the root of this branch.")
 
+    def _get_config(self):
+        return TransportConfig(self._transport, 'branch.conf')
+
     def is_locked(self):
         return self.control_files.is_locked()
 

=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/config.py	2009-04-02 03:37:30 +0000
@@ -709,7 +709,6 @@
                         trace.warning('Value "%s" is masked by "%s" from'
                                       ' branch.conf', value, mask_value)
 
-
     def _gpg_signing_command(self):
         """See Config.gpg_signing_command."""
         return self._get_safe_value('_gpg_signing_command')
@@ -917,10 +916,7 @@
     # XXX: Really needs a better name, as this is not part of the tree! -- mbp 20080507
 
     def __init__(self, branch):
-        # XXX: Really this should be asking the branch for its configuration
-        # data, rather than relying on a Transport, so that it can work
-        # more cleanly with a RemoteBranch that has no transport.
-        self._config = TransportConfig(branch._transport, 'branch.conf')
+        self._config = branch._get_config()
         self.branch = branch
 
     def _get_parser(self, file=None):

=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py	2009-04-01 05:45:58 +0000
+++ b/bzrlib/remote.py	2009-04-02 03:37:30 +0000
@@ -22,6 +22,7 @@
 from bzrlib import (
     branch,
     bzrdir,
+    config,
     debug,
     errors,
     graph,
@@ -1937,6 +1938,9 @@
             return
         self._activate_fallback_location(fallback_url)
 
+    def _get_config(self):
+        return RemoteBranchConfig(self)
+
     def _get_real_transport(self):
         # if we try vfs access, return the real branch's vfs transport
         self._ensure_real()
@@ -2355,6 +2359,60 @@
         return self._real_branch.set_push_location(location)
 
 
+class RemoteBranchConfig(object):
+    """A Config that reads from a smart branch and writes via smart methods.
+
+    It is a low-level object that considers config data to be name/value pairs
+    that may be associated with a section. Assigning meaning to the these
+    values is done at higher levels like bzrlib.config.TreeConfig.
+    """
+
+    def __init__(self, branch):
+        self._branch = branch
+
+    def get_option(self, name, section=None, default=None):
+        """Return the value associated with a named option.
+
+        :param name: The name of the value
+        :param section: The section the option is in (if any)
+        :param default: The value to return if the value is not set
+        :return: The value or default value
+        """
+        configobj = self._get_configobj()
+        if section is None:
+            section_obj = configobj
+        else:
+            try:
+                section_obj = configobj[section]
+            except KeyError:
+                return default
+        return section_obj.get(name, default)
+
+    def _get_configobj(self):
+        path = self._branch.bzrdir._path_for_remote_call(
+            self._branch._client)
+        response = self._branch._client.call_expecting_body(
+            'Branch.get_config_file', path)
+        if response[0][0] != 'ok':
+            raise UnexpectedSmartServerResponse(response)
+        bytes = response[1].read_body_bytes()
+        return config.ConfigObj([bytes], encoding='utf-8')
+
+    def set_option(self, value, name, section=None):
+        """Set the value associated with a named option.
+
+        :param value: The value to set
+        :param name: The name of the value to set
+        :param section: The section the option is in (if any)
+        """
+        return self._vfs_set_option(value, name, section)
+
+    def _vfs_set_option(self, value, name, section=None):
+        self._branch._ensure_real()
+        return self._branch._real_branch._get_config().set_option(
+            value, name, section)
+
+
 def _extract_tar(tar, to_dir):
     """Extract all the contents of a tarfile object.
 

=== modified file 'bzrlib/smart/branch.py'
--- a/bzrlib/smart/branch.py	2009-03-24 06:40:26 +0000
+++ b/bzrlib/smart/branch.py	2009-04-02 03:37:30 +0000
@@ -80,11 +80,6 @@
 
         The body is not utf8 decoded - its the literal bytestream from disk.
         """
-        # This was at one time called by RemoteBranchLockableFiles
-        # intercepting access to this file; as of 1.5 it is not called by the
-        # client but retained for compatibility.  It may be called again to
-        # allow the client to get the configuration without needing vfs
-        # access.
         try:
             content = branch._transport.get_bytes('branch.conf')
         except errors.NoSuchFile:

=== modified file 'bzrlib/tests/blackbox/test_branch.py'
--- a/bzrlib/tests/blackbox/test_branch.py	2009-04-01 03:53:19 +0000
+++ b/bzrlib/tests/blackbox/test_branch.py	2009-04-02 04:01:23 +0000
@@ -267,13 +267,12 @@
         self.reset_smart_call_log()
         out, err = self.run_bzr(['branch', self.get_url('from'),
             self.get_url('target')])
-        rpc_count = len(self.hpss_calls)
         # This figure represent the amount of work to perform this use case. It
         # is entirely ok to reduce this number if a test fails due to rpc_count
         # being too low. If rpc_count increases, more network roundtrips have
         # become necessary for this use case. Please do not adjust this number
         # upwards without agreement from bzr's network support maintainers.
-        self.assertEqual(53, rpc_count)
+        self.assertLength(53, self.hpss_calls)
 
     def test_branch_from_trivial_branch_streaming_acceptance(self):
         self.setup_smart_server_with_call_log()
@@ -283,13 +282,12 @@
         self.reset_smart_call_log()
         out, err = self.run_bzr(['branch', self.get_url('from'),
             'local-target'])
-        rpc_count = len(self.hpss_calls)
         # This figure represent the amount of work to perform this use case. It
         # is entirely ok to reduce this number if a test fails due to rpc_count
         # being too low. If rpc_count increases, more network roundtrips have
         # become necessary for this use case. Please do not adjust this number
         # upwards without agreement from bzr's network support maintainers.
-        self.assertEqual(10, rpc_count)
+        self.assertLength(10, self.hpss_calls)
 
     def test_branch_from_trivial_stacked_branch_streaming_acceptance(self):
         self.setup_smart_server_with_call_log()

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/test_config.py	2009-04-02 05:05:17 +0000
@@ -150,6 +150,9 @@
         self._transport = self.control_files = \
             FakeControlFilesAndTransport(user_id=user_id)
 
+    def _get_config(self):
+        return config.TransportConfig(self._transport, 'branch.conf')
+
     def lock_write(self):
         pass
 

=== modified file 'bzrlib/tests/test_remote.py'
--- a/bzrlib/tests/test_remote.py	2009-04-01 04:05:29 +0000
+++ b/bzrlib/tests/test_remote.py	2009-04-02 03:37:30 +0000
@@ -1332,30 +1332,30 @@
         self.assertEqual('rejection message', err.msg)
 
 
-class TestBranchControlGetBranchConf(tests.TestCaseWithMemoryTransport):
+class TestBranchControlGetBranchConf(RemoteBranchTestCase):
     """Getting the branch configuration should use an abstract method not vfs.
     """
 
     def test_get_branch_conf(self):
-        raise tests.KnownFailure('branch.conf is not retrieved by get_config_file')
-        ## # We should see that branch.get_config() does a single rpc to get the
-        ## # remote configuration file, abstracting away where that is stored on
-        ## # the server.  However at the moment it always falls back to using the
-        ## # vfs, and this would need some changes in config.py.
+        # We should see that branch.get_config() does a single rpc to get the
+        # remote configuration file, abstracting away where that is stored on
+        # the server.  However at the moment it always falls back to using the
+        # vfs, and this would need some changes in config.py.
 
-        ## # in an empty branch we decode the response properly
-        ## client = FakeClient([(('ok', ), '# config file body')], self.get_url())
-        ## # we need to make a real branch because the remote_branch.control_files
-        ## # will trigger _ensure_real.
-        ## branch = self.make_branch('quack')
-        ## transport = branch.bzrdir.root_transport
-        ## # we do not want bzrdir to make any remote calls
-        ## bzrdir = RemoteBzrDir(transport, _client=False)
-        ## branch = RemoteBranch(bzrdir, None, _client=client)
-        ## config = branch.get_config()
-        ## self.assertEqual(
-        ##     [('call_expecting_body', 'Branch.get_config_file', ('quack/',))],
-        ##     client._calls)
+        # in an empty branch we decode the response properly
+        client = FakeClient()
+        client.add_expected_call(
+            'Branch.get_stacked_on_url', ('memory:///',),
+            'error', ('NotStacked',),)
+        client.add_success_response_with_body('# config file body', 'ok')
+        transport = MemoryTransport()
+        branch = self.make_remote_branch(transport, client)
+        config = branch.get_config()
+        config.has_explicit_nickname()
+        self.assertEqual(
+            [('call', 'Branch.get_stacked_on_url', ('memory:///',)),
+             ('call_expecting_body', 'Branch.get_config_file', ('memory:///',))],
+            client._calls)
 
 
 class TestBranchLockWrite(RemoteBranchTestCase):




More information about the bazaar-commits mailing list