HPSS: Client-side objects

[Aaron Bentley]

Andrew Bennetts wrote:
> +    def open_repository(self):
> +        path = self._path_for_remote_call(self._client)
> +        response = self._client.call('BzrDir.find_repository', path)
> +        assert response[0] in ('ok', 'norepository'), \
> +            'unexpected response code %s' % (response,)
> +        if response[0] == 'norepository':
> +            raise errors.NoRepositoryPresent(self)
> +        assert len(response) == 4, 'incorrect response length %s' % (response,)
> +        if response[1] == '':
> +            format = RemoteRepositoryFormat()
> +            format.rich_root_data = response[2] == 'True'

^^^ It seems the HPSS is returning 'ok', 'yes', 'no', 'True' (and
presumably 'False') for boolean values.  I think perhaps you should
standardize.

Also, the x = y == z is not especially clear.  I think we recommend
surrounding the boolean expression with parentheses, e.g. x = (y == z)

> +    def can_convert_format(self):
> +        """Upgrading of remote bzrdirs is not supported yet."""
> +        return False

Should we consider supporting the VFS upgrade approach?

> +class RemoteBranch(branch.Branch):

^^^^ Hmm.  Didn't we already have a class named this?  Oh yeah, back
around revno 1390 :-)

> +    @needs_write_lock
> +    def set_revision_history(self, rev_history):
> +        # Send just the tip revision of the history; the server will generate
> +        # the full history from that.  If the revision doesn't exist in this
> +        # branch, NoSuchRevision will be raised.

This is not good enough, because we don't have a guarantee that the
caller will provide the lefthand history.  See the assertions in
Branch6.set_revision_history.

> +        path = self.bzrdir._path_for_remote_call(self._client)
> +        if rev_history == []:
> +            rev_id = ''

^^^ I think NULL_REVISION/'null:' is much better than '' for this purpose.

> === modified file 'bzrlib/branch.py'
> --- bzrlib/branch.py	2007-04-13 07:18:57 +0000
> +++ bzrlib/branch.py	2007-04-16 05:57:08 +0000
> @@ -579,7 +579,8 @@
>  
>      def get_push_location(self):
>          """Return the None or the location to push this branch to."""
> -        raise NotImplementedError(self.get_push_location)bzrlib/branch.py
> +        push_loc = self.get_config().get_user_option('push_location')
> +        return push_loc

^^^ I thought we were leaving details like this to particular
implementations.

>      def _get_checkout_format(self):
>          """Return the most suitable metadir for a checkout of this branch.
> -        Weaves are used if this branch's repostory uses weaves.
> +        Weaves are used if this branch's repository uses weaves.
>          """
> +        from bzrlib.remote import RemoteBzrDir
>          if isinstance(self.bzrdir, bzrdir.BzrDirPreSplitOut):
>              from bzrlib.repofmt import weaverepo
>              format = bzrdir.BzrDirMetaFormat1()
>              format.repository_format = weaverepo.RepositoryFormat7()
> +        elif isinstance(self.bzrdir, RemoteBzrDir):
> +            format = bzrdir.BzrDirMetaFormat1()

^^^ For a RemoteBzrDir, we should be using the same formats as the
remote branch uses.  Can't we at least fall back to the VFS stuff?

> +# We can't use register_control_format because it adds it at a lower priority
> +# than the existing branches, whereas this should take priority.

Can't we just register it first, then?

Aaron