How to create new methods

[Martin Pool]

On 4/25/07, John Arbash Meinel <john at arbash-meinel.com> wrote:
> I have a question about how to create new methods, now that the smart
> server has landed (with more intelligent Remote objects).
>
> Specifically, I'm looking to create a "get_partial_revision_graph()"
> function on Repository, and RemoteRepository does not inherit from any
> base class. So from the first part, I would have to just copy and paste
> the same implementation so that all tests pass. But any time I copy &
> paste, I'm sure there is something wrong.

You should just be able to add something like this:

    def fileids_altered_by_revision_ids(self, revision_ids):
        self._ensure_real()
        return self._real_repository.fileids_altered_by_revision_ids(revision_ids)

This is how we fall back to using the vfs-based implementation.

There is still some repetition here and we could maybe autogenerate
all these stubs.

> This function is a (potentially) good candidate for having a new verb,
> because given two nodes, you don't have to download the complete graph.
> Rather than the simple implementation, which grabs the full graph, and
> removes a subset.

Agree.

> It would be nice (for compatibility) if we could have a way to "try the
> new function, if not available, fall back to the slow method".

As you indicate below you'd want something like

  try:
     return self._client._call('Repostiory.partial_graph')
  except UnknownSmartVerb:
    ensure_real
    self._real_repository.partial_graph()

but that depends on that error being clearly returned to and
identified by the client - Robert and Andrew's recent patch for errors
goes towards that.

> It isn't terrible to write this, though the inheritance hierarchy means
> I need to write them as a non-member function.
>
> I'm just trying to figure out what the best way to handle this is. Since
> having an if/then or try/catch in every smart function (to fall back to
> the VFS version) seems ugly (code wise). Rather than having something
> more automatic. Like a decorator:
>
> @needs_verbs(RemoteRepository.get_graph_subset)
> def get_partial_revision_graph(self, start, end):
> ...
>
> @needs_no_verbs
> def get_partial_revision_graph(self, start, end):
> ...
>
> Anyway, I don't really know the "best" thing. I'm just trying to come up
> with ways to allow cross-version compatibility without having the code
> become a mess of try/except.

How about this:

One of the proposed changes has the client ask the server its version
when it first connects.  Since it's a stateless connection we can't be
sure  this won't change but it would be cruel for the server to be
downgraded.  So we should actually know pretty well at that point what
we can rely on.  We can know what verbs are available in each bzr
release, and construct a different RemoteRepository object depending
on what's available.

This might cause a proliferation of those classes though...

Alternatively: it may be a reasonable requirement while we are in this
phase that the server be no older than the client, in which case you
could just go ahead and call it.

-- 
Martin