HPSS: new server commands

Andrew Bennetts andrew at canonical.com
Mon Apr 16 18:23:18 BST 2007


Aaron Bentley wrote:
> Andrew Bennetts wrote:
> > +class SmartServerBranchRequestLastRevisionInfo(SmartServerBranchRequest):
> > +    
> > +    def do_with_branch(self, branch):
> > +        """Return branch.last_revision_info().
[...]
> > +
> > +
> > +class SmartServerBranchRequestSetLastRevision(SmartServerLockedBranchRequest):
> > +    
> > +    def do_with_locked_branch(self, branch, new_last_revision_id):
> > +        if new_last_revision_id == '':
> > +            branch.set_revision_history([])
[...]
> 
> I think it makes more sense to implement SetLastRevisionInfo.  This maps
> directly to Branch.set_last_revision_info, and runs in O(1) time on a
> Branch6 branch.  (Whereas any implementation of SetLastRevision will
> always run in O(n) time, on either branch type)

Branch6 didn't exist when this was written IIRC, but that sounds like a good
improvement.  I'm not going to have time to do it before the 0.16 freeze though.
This at least will avoid sending the entire branch revision-history just to set
the last revision on pre-format 6 branches, which is still a significant win.

> > +class SmartServerRequestHasRevision(SmartServerRepositoryRequest):
> > +
> > +    def do_repository_request(self, repository, revision_id):
> > +        """Return ok if a specific revision is in the repository at path.
> > +
> > +        :param repository: The repository to query in.
> > +        :param revision_id: The utf8 encoded revision_id to lookup.
> > +        :return: A smart server response of ('ok', ) if the revision is
> > +            present.
> > +        """
> > +        if repository.has_revision(revision_id):
> > +            return SmartServerResponse(('ok', ))
> > +        else:
> > +            return SmartServerResponse(('no', ))
> 
> Does this make sense?  To me, 'ok' means the request was valid, and was
> successfully processed.  So I'd expect something more like:

Yeah, the return values are currently a bit inconsistent.  I'll fix this one to
use 'yes'/'no' like SmartServerRepositoryIsShared.  Martin has specced out a
consistent scheme we'll switch to fairly soon (part of which is that the first
element of the response will always be a status code, either 'ok' or 'error').

Thanks for the review!

-Andrew.




More information about the bazaar mailing list