[MERGE] Re: knits - slow sftp pushes

[Martin Pool]

On  2 May 2006, Robert Collins <robertc at robertcollins.net> wrote:
> Anyhow, I've done this (and realised I hadn't completely forgotten it -
> there was a TODO - I'd just underestimated *how* bad it would be).
> 
> also up at http://people.ubuntu.com/~robertc/baz2.0/knit-join-batching/.

+1 with some comments, thankyou.

> +    def add_versions(self, versions):
> +        """Add multiple versions to the index.
> +        
> +        :param versions: a list of tuples:
> +                         (version_id, options, pos, size, parents).
> +        """
> +        lines = []
> +        for version_id, options, pos, size, parents in versions:
> +            line = "\n%s %s %s %s %s :" % (version_id.encode('utf-8'),
> +                                           ','.join(options),
> +                                           pos,
> +                                           size,
> +                                           self._version_list_to_index(parents))
> +            assert isinstance(line, str), 'content must be utf-8 encoded'
> +            lines.append(line)

Including the data that caused the problem encourages better bug
reports, so

               assert isinstance(line, str), \
		  'content must be utf-8 encoded: %r' % (line,)

> +        self._transport.append(self._filename, StringIO(''.join(lines)))
> +        # cache after writing, so that a failed write leads to missing cache
> +        # entries not extra ones. XXX TODO: RBC 20060502 in the event of a 
> +        # failure, reload the index or flush it or some such, to prevent
> +        # writing records that did complete twice.
> +        for version_id, options, pos, size, parents in versions:
> +            self._cache_version(version_id, options, pos, size, parents)
> +        
>      def has_version(self, version_id):
>          """True if the version is in the index."""
>          return self._cache.has_key(version_id)
> @@ -1237,8 +1276,7 @@
>      def add_raw_record(self, raw_data):
>          """Append a prepared record to the data file."""
>          assert isinstance(raw_data, str), 'data must be plain bytes'
> -        start_pos = self._transport.append(self._filename, StringIO(raw_data))
> -        return start_pos, len(raw_data)
> +        return self._transport.append(self._filename, StringIO(raw_data))

This changes the return type.  I see one caller is updated, but are
there any others.  Does anyone actually use the return value at all?
Please either document it or just remove it.

-- 
Martin