[MERGE] Better infrastructure for tracking remote server version in SmartClientMedium

[Martin Pool]

On Wed, Jun 11, 2008 at 3:05 PM, Andrew Bennetts <andrew at canonical.com> wrote:
> Martin Pool wrote:
>> Martin Pool has voted resubmit.
>> Status is now: Resubmit
>> Comment:
>>> .... keep
>> > adding _remote_is_at_least_1_6 and trying to keep them all consistent
>> with each
>>
>> That's a noble goal.
>>
>> However I feel like this is getting the same kind of problem with regard
>> to the operators applied to these formats: !=, >=, etc are all being
>> spelled out as words and it's not clear to me that they will always be
>> consistent; or at least there seems a fair risk that they will become
>> inconsistent with future evolution.
>
> I don't much like these method names much.  I'm open to suggestions for
> improvement.

The other thing that was bothering me is that I think the client
actually knows the server is in a particular interval of versions: at
least X, but no more than Y.  These methods seem a bit confused about
that.

>> If we want to have an api that says what the remote version is I'd
>> rather just expose the actual version tuple, or at least the approximate
>> version tuple.
>
> Well, I prefer a method for assignment at least, because I want to enforce
> ratcheting (if only for the sake of clarity rather than fear of bugs).  I'd be
> ok with doing the <= comparison against a tuple retrieved via a getter method or
> read-only property.

Right.

>
>> But more broadly, I thought our approach was that we wanted to avoid
>> having the client making decisions based on the server version, because
>> it's problematic with stateless http and seems to require a startup
>> roundtrip which we wanted to avoid.  Maybe we want to reconsider it? But
>> if it's just that we know the server doesn't support a particular
>> operation or encoding, maybe we should record that fact specifically?
>
> Well, we need some way to gracefully cope with newer clients talking to older
> servers.  The current approach of just optimistically trying the highest version
> then falling back, and remember that you had to fall back on a per-connection
> basis, seems to be the least worst option.
>
> We could remember that individual verbs aren't available, but that seems like a
> YAGNI to me, especially considering it would add complexity to give worse
> performance (if one 1.2 verb is not supported, why waste time trying another?).
>
> An added benefit of this approach is that it makes it easy to say "if the
> protocol version is 2, we can immediately assume the remote only supports verbs
> from 1.2 and earlier".  It also makes it easy to add a
> Remote-objects-with-a-no-1.6-verbs-server scenario to
> bzrdir/branch/repository-implementations along side the existing
> Remote-objects-with-current-server scenario (I have a patch to add exactly that
> that I'm about to submit now that I've got tests passing in a dependent loom
> thread).

Yes this does seem to touch on systematically knowing which version
added which verbs and testing across the facilities available in
earlier versions.

If we are going to remember the server's version as a number across
replies maybe it's better to just make use of the fact that in
protocol 3 it's given explicitly, rather than ignoring that and
inferring it?

-- 
Martin <http://launchpad.net/~mbp/>