Rev 2230: Deal with various review comments from Robert. in sftp://bazaar.launchpad.net/%7Ebzr/bzr/hpss/

Andrew Bennetts andrew.bennetts at canonical.com
Tue Apr 17 10:03:37 BST 2007


At sftp://bazaar.launchpad.net/%7Ebzr/bzr/hpss/

------------------------------------------------------------
revno: 2230
revision-id: andrew.bennetts at canonical.com-20070417090153-a7apw9czzedpt4nk
parent: andrew.bennetts at canonical.com-20070417043051-yt3bxfhm85byfntc
committer: Andrew Bennetts <andrew.bennetts at canonical.com>
branch nick: hpss
timestamp: Tue 2007-04-17 19:01:53 +1000
message:
  Deal with various review comments from Robert.
modified:
  bzrlib/bzrdir.py               bzrdir.py-20060131065624-156dfea39c4387cb
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/fetch.py                fetch.py-20050818234941-26fea6105696365d
  bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
  bzrlib/repository.py           rev_storage.py-20051111201905-119e9401e46257e3
  bzrlib/smart/bzrdir.py         bzrdir.py-20061122024551-ol0l0o0oofsu9b3t-1
  bzrlib/smart/request.py        request.py-20061108095550-gunadhxmzkdjfeek-1
  bzrlib/tests/test_errors.py    test_errors.py-20060210110251-41aba2deddf936a8
  bzrlib/tests/test_remote.py    test_remote.py-20060720103555-yeeg2x51vn0rbtdp-2
=== modified file 'bzrlib/bzrdir.py'
--- a/bzrlib/bzrdir.py	2007-04-16 17:49:35 +0000
+++ b/bzrlib/bzrdir.py	2007-04-17 09:01:53 +0000
@@ -1377,6 +1377,17 @@
         klass._control_formats.append(format)
 
     @classmethod
+    def register_control_server_format(klass, format):
+        """Register a control format for client-server environments.
+
+        These formats will be tried before ones registered with
+        register_control_format.  This gives implementations that decide to the
+        chance to grab it before anything looks at the contents of the format
+        file.
+        """
+        klass._control_formats.insert(0, format)
+
+    @classmethod
     @symbol_versioning.deprecated_method(symbol_versioning.zero_fourteen)
     def set_default_format(klass, format):
         klass._set_default_format(format)
@@ -2237,9 +2248,7 @@
         return self.get_format_description() == other.get_format_description()
 
 
-# We can't use register_control_format because it adds it at a lower priority
-# than the existing branches, whereas this should take priority.
-BzrDirFormat._control_formats.insert(0, RemoteBzrDirFormat)
+BzrDirFormat.register_control_server_format(RemoteBzrDirFormat)
 
 
 class BzrDirFormatInfo(object):

=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2007-04-13 06:23:59 +0000
+++ b/bzrlib/errors.py	2007-04-17 09:01:53 +0000
@@ -2067,3 +2067,12 @@
 
     def __init__(self, tag_name):
         self.tag_name = tag_name
+
+
+class UnexpectedSmartServerResponse(BzrError):
+
+    _fmt = "Could not understand response from smart server: %(response_tuple)r"
+
+    def __init__(self, response_tuple):
+        self.response_tuple = response_tuple
+

=== modified file 'bzrlib/fetch.py'
--- a/bzrlib/fetch.py	2007-03-06 13:36:49 +0000
+++ b/bzrlib/fetch.py	2007-04-17 09:01:53 +0000
@@ -92,12 +92,6 @@
         # result variables.
         self.failed_revisions = []
         self.count_copied = 0
-        if isinstance(to_repository, RemoteRepository):
-            to_repository._ensure_real()
-            to_repository = to_repository._real_repository
-        if isinstance(from_repository, RemoteRepository):
-            from_repository._ensure_real()
-            from_repository = from_repository._real_repository
         if to_repository.control_files._transport.base == from_repository.control_files._transport.base:
             # check that last_revision is in 'from' and then return a no-operation.
             if last_revision not in (None, NULL_REVISION):

=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py	2007-04-17 04:30:51 +0000
+++ b/bzrlib/remote.py	2007-04-17 09:01:53 +0000
@@ -34,7 +34,7 @@
 # Note: RemoteBzrDirFormat is in bzrdir.py
 
 class RemoteBzrDir(BzrDir):
-    """Control directory on a remote server, accessed by HPSS."""
+    """Control directory on a remote server, accessed via bzr:// or similar."""
 
     def __init__(self, transport, _client=None):
         """Construct a RemoteBzrDir.
@@ -45,8 +45,6 @@
         BzrDir.__init__(self, transport, RemoteBzrDirFormat())
         # this object holds a delegated bzrdir that uses file-level operations
         # to talk to the other side
-        # XXX: We should go into find_format, but not allow it to find
-        # RemoteBzrDirFormat and make sure it finds the real underlying format.
         self._real_bzrdir = None
 
         if _client is None:
@@ -59,17 +57,21 @@
 
         self._ensure_real()
         path = self._path_for_remote_call(self._client)
-        #self._real_bzrdir._format.probe_transport(transport)
-        response = self._client.call('probe_dont_use', path)
+        response = self._client.call('BzrDir.open', path)
+        if response not in [('yes',), ('no',)]:
+            raise errors.UnexpectedSmartServerResponse(response)
         if response == ('no',):
             raise errors.NotBranchError(path=transport.base)
 
     def _ensure_real(self):
         """Ensure that there is a _real_bzrdir set.
 
-        used before calls to self._real_bzrdir.
+        Used before calls to self._real_bzrdir.
         """
         if not self._real_bzrdir:
+            # XXX: We can't use BzrDir.open_from_transport here because it
+            # causes infinite recursion, so just try opening the bzrdir with the
+            # default format.
             default_format = BzrDirFormat.get_default_format()
             self._real_bzrdir = default_format.open(self.root_transport,
                 _found=True)
@@ -216,8 +218,8 @@
 class RemoteRepository(object):
     """Repository accessed over rpc.
 
-    For the moment everything is delegated to IO-like operations over
-    the transport.
+    For the moment most operations are performed using local transport-backed
+    Repository objects.
     """
 
     def __init__(self, remote_bzrdir, format, real_repository=None, _client=None):
@@ -249,7 +251,7 @@
     def _ensure_real(self):
         """Ensure that there is a _real_repository set.
 
-        used before calls to self._real_repository.
+        Used before calls to self._real_repository.
         """
         if not self._real_repository:
             self.bzrdir._ensure_real()
@@ -267,7 +269,8 @@
         assert type(revision_id) is str
         response = self._client.call_expecting_body(
             'Repository.get_revision_graph', path, revision_id)
-        assert response[0][0] in ('ok', 'nosuchrevision'), 'unexpected response code %s' % (response[0],)
+        if response[0][0] not in ['ok', 'nosuchrevision']:
+            raise errors.UnexpectedSmartServerResponse(response[0])
         if response[0][0] == 'ok':
             coded = response[1].read_body_bytes()
             if coded == '':
@@ -275,7 +278,6 @@
                 return {}
             lines = coded.split('\n')
             revision_graph = {}
-            # FIXME
             for line in lines:
                 d = list(line.split())
                 revision_graph[d[0]] = d[1:]
@@ -419,6 +421,8 @@
             if self._real_repository is not None:
                 self._real_repository.unlock()
             if mode != 'w':
+                # Only write-locked repositories need to make a remote method
+                # call to perfom the unlock.
                 return
             assert self._lock_token, 'Locked, but no token!'
             token = self._lock_token
@@ -617,10 +621,8 @@
         self.bzrdir = bzrdir
         self._client = _client
         self._need_find_modes = True
-        # XXX: This assumes that the branch control directory is .bzr/branch,
-        # which isn't necessarily true.
         LockableFiles.__init__(
-            self, bzrdir.root_transport.clone('.bzr/branch'),
+            self, bzrdir.get_branch_transport(None),
             'lock', lockdir.LockDir)
 
     def _find_modes(self):
@@ -682,7 +684,9 @@
             format, usually accessing the data via the VFS.
         :param _client: Private parameter for testing.
         """
-        #branch.Branch.__init__(self)
+        # We intentionally don't call the parent class's __init__, because it
+        # will try to assign to self.tags, which is a property in this subclass.
+        # And the parent's __init__ doesn't do much anyway.
         self._revision_history_cache = None
         self.bzrdir = remote_bzrdir
         if _client is not None:
@@ -714,7 +718,7 @@
     def _ensure_real(self):
         """Ensure that there is a _real_branch set.
 
-        used before calls to self._real_branch.
+        Used before calls to self._real_branch.
         """
         if not self._real_branch:
             assert vfs.vfs_enabled()
@@ -836,6 +840,8 @@
                     self._real_branch.repository.leave_lock_in_place()
                 self._real_branch.unlock()
             if mode != 'w':
+                # Only write-locked branched need to make a remote method call
+                # to perfom the unlock.
                 return
             assert self._lock_token, 'Locked, but no token!'
             branch_token = self._lock_token

=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py	2007-04-13 06:23:59 +0000
+++ b/bzrlib/repository.py	2007-04-17 09:01:53 +0000
@@ -34,6 +34,7 @@
     lockdir,
     osutils,
     registry,
+    remote,
     revision as _mod_revision,
     symbol_versioning,
     transactions,
@@ -1689,11 +1690,51 @@
         return f.count_copied, f.failed_revisions
 
 
+class InterRemoteRepository(InterRepository):
+    """Code for converting between RemoteRepository objects.
+
+    This just gets an non-remote repository from the RemoteRepository, and calls
+    InterRepository.get again.
+    """
+
+    def __init__(self, source, target):
+        if isinstance(source, remote.RemoteRepository):
+            source._ensure_real()
+            real_source = source._real_repository
+        else:
+            real_source = source
+        if isinstance(target, remote.RemoteRepository):
+            target._ensure_real()
+            real_target = target._real_repository
+        else:
+            real_target = target
+        self.real_inter = InterRepository.get(real_source, real_target)
+
+    @staticmethod
+    def is_compatible(source, target):
+        if isinstance(source, remote.RemoteRepository):
+            return True
+        if isinstance(target, remote.RemoteRepository):
+            return True
+        return False
+
+    def copy_content(self, revision_id=None):
+        self.real_inter.copy_content(revision_id=revision_id)
+
+    def fetch(self, revision_id=None, pb=None):
+        self.real_inter.fetch(revision_id=revision_id, pb=pb)
+
+    @classmethod
+    def _get_repo_format_to_test(self):
+        return None
+
+
 InterRepository.register_optimiser(InterSameDataRepository)
 InterRepository.register_optimiser(InterWeaveRepo)
 InterRepository.register_optimiser(InterKnitRepo)
 InterRepository.register_optimiser(InterModel1and2)
 InterRepository.register_optimiser(InterKnit1and2)
+InterRepository.register_optimiser(InterRemoteRepository)
 
 
 class RepositoryTestProviderAdapter(object):

=== modified file 'bzrlib/smart/bzrdir.py'
--- a/bzrlib/smart/bzrdir.py	2007-04-05 09:35:26 +0000
+++ b/bzrlib/smart/bzrdir.py	2007-04-17 09:01:53 +0000
@@ -22,6 +22,22 @@
 from bzrlib.smart.request import SmartServerRequest, SmartServerResponse
 
 
+class SmartServerRequestOpenBzrDir(SmartServerRequest):
+
+    def do(self, path):
+        from bzrlib.bzrdir import BzrDirFormat
+        t = self._backing_transport.clone(path)
+        default_format = BzrDirFormat.get_default_format()
+        real_bzrdir = default_format.open(t, _found=True)
+        try:
+            real_bzrdir._format.probe_transport(t)
+        except (errors.NotBranchError, errors.UnknownFormatError):
+            answer = 'no'
+        else:
+            answer = 'yes'
+        return SmartServerResponse((answer,))
+
+
 class SmartServerRequestFindRepository(SmartServerRequest):
 
     def do(self, path):

=== modified file 'bzrlib/smart/request.py'
--- a/bzrlib/smart/request.py	2007-04-16 06:53:00 +0000
+++ b/bzrlib/smart/request.py	2007-04-17 09:01:53 +0000
@@ -221,25 +221,6 @@
         return SmartServerResponse((), tmpf.read())
 
 
-# This exists solely to help RemoteObjectHacking.  It should be removed
-# eventually.  It should not be considered part of the real smart server
-# protocol!
-class ProbeDontUseRequest(SmartServerRequest):
-
-    def do(self, path):
-        from bzrlib.bzrdir import BzrDirFormat
-        t = self._backing_transport.clone(path)
-        default_format = BzrDirFormat.get_default_format()
-        real_bzrdir = default_format.open(t, _found=True)
-        try:
-            real_bzrdir._format.probe_transport(t)
-        except (errors.NotBranchError, errors.UnknownFormatError):
-            answer = 'no'
-        else:
-            answer = 'yes'
-        return SmartServerResponse((answer,))
-
-
 class SmartServerIsReadonly(SmartServerRequest):
     # XXX: this request method belongs somewhere else.
 
@@ -318,4 +299,4 @@
 request_handlers.register_lazy(
     'Transport.is_readonly', 'bzrlib.smart.request', 'SmartServerIsReadonly')
 request_handlers.register_lazy(
-    'probe_dont_use', 'bzrlib.smart.request', 'ProbeDontUseRequest')
+    'BzrDir.open', 'bzrlib.smart.bzrdir', 'SmartServerRequestOpenBzrDir')

=== modified file 'bzrlib/tests/test_errors.py'
--- a/bzrlib/tests/test_errors.py	2007-04-11 04:26:23 +0000
+++ b/bzrlib/tests/test_errors.py	2007-04-17 09:01:53 +0000
@@ -233,6 +233,11 @@
             host='ahost', port=444, msg='Unable to connect to ssh host',
             orig_error='my_error')
 
+    def test_unexpected_smart_server_response(self):
+        e = errors.UnexpectedSmartServerResponse(('not yes',))
+        self.assertEqual(
+            "Could not understand response from smart server: ('not yes',)",
+            str(e))
 
 
 class PassThroughError(errors.BzrError):

=== modified file 'bzrlib/tests/test_remote.py'
--- a/bzrlib/tests/test_remote.py	2007-04-16 17:49:35 +0000
+++ b/bzrlib/tests/test_remote.py	2007-04-17 09:01:53 +0000
@@ -68,6 +68,7 @@
         # open a standalone branch in the working directory
         b = remote.RemoteBzrDir(self.transport)
         branch = b.open_branch()
+        self.assertIsInstance(branch, Branch)
 
     def test_remote_repository(self):
         b = BzrDir.open_from_transport(self.transport)




More information about the bazaar-commits mailing list