Rev 3107: Jam's and Aaron feedback about bug #175886. in file:///v/home/vila/src/bzr/bugs/175886/

[Vincent Ladeuil]

At file:///v/home/vila/src/bzr/bugs/175886/

------------------------------------------------------------
revno: 3107
revision-id:v.ladeuil+lp at free.fr-20071212210605-utjudvizj5v9qotc
parent: v.ladeuil+lp at free.fr-20071212161401-uo6j6ohcwkbj1h4p
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 175886
timestamp: Wed 2007-12-12 22:06:05 +0100
message:
  Jam's and Aaron feedback about bug #175886.
  
  * bzrlib/transport/http/_urllib2_wrappers.py:
  (Response.finish): Returned the number of bytes discared.
  (AbstractHTTPConnection.cleanup_pipe): Warn the user about servers
  that do not recognize range requests.
modified:
  bzrlib/transport/http/__init__.py http_transport.py-20050711212304-506c5fd1059ace96
  bzrlib/transport/http/_urllib2_wrappers.py _urllib2_wrappers.py-20060913231729-ha9ugi48ktx481ao-1
-------------- next part --------------
=== modified file 'bzrlib/transport/http/__init__.py'
--- a/bzrlib/transport/http/__init__.py	2007-12-10 10:41:24 +0000
+++ b/bzrlib/transport/http/__init__.py	2007-12-12 21:06:05 +0000
@@ -139,7 +139,6 @@
         # time of this writing it's even the only known client -- vila20071203
         return StringIO(response_file.read())
 
-    # TODO: Add tests for tail_amount or deprecate it
     def _get(self, relpath, ranges, tail_amount=0):
         """Get a file, or part of a file.
 

=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
--- a/bzrlib/transport/http/_urllib2_wrappers.py	2007-12-12 15:43:35 +0000
+++ b/bzrlib/transport/http/_urllib2_wrappers.py	2007-12-12 21:06:05 +0000
@@ -123,6 +123,12 @@
             # below we keep the socket with the server opened.
             self.will_close = False
 
+    # in finish() below, we may have to discard several MB in the worst
+    # case. To avoid buffering that much, we read and discard by chunks
+    # instead. The underlying file is either a socket or a StringIO, so reading
+    # 8k chunks should be fine.
+    _discarded_buf_size = 8192
+
     def finish(self):
         """Finish reading the body.
 
@@ -131,17 +137,15 @@
         persistent connection. If we don't use a persistent connection, well,
         nothing will block the next request since a new connection will be
         issued anyway.
+
+        :return: the number of bytes left on the socket (may be None)
         """
+        pending = None
         if not self.isclosed():
             # Make sure nothing was left to be read on the socket
             pending = 0
-            # We can't read all the remaining data in a single read since there
-            # may be a lot to read (several MB in the worst cases). We read and
-            # discard by chunks instead. The underlying file is either a socket
-            # or a StringIO, so reading 8k chunks should be fine.
-            bufsiz = 8192
-            while self.length and self.length > bufsiz:
-                data = self.read(bufsiz)
+            while self.length and self.length > self._discarded_buf_size:
+                data = self.read(self._discarded_buf_size)
                 pending += len(data)
             if self.length:
                 data = self.read(self.length)
@@ -152,6 +156,7 @@
                     "%s bytes left on the socket",
                     pending)
             self.close()
+        return pending
 
 
 # Not inheriting from 'object' because httplib.HTTPConnection doesn't.
@@ -161,8 +166,13 @@
     response_class = Response
     strict = 1 # We don't support HTTP/0.9
 
+    # When we detect a server responding with the whole file to range requests,
+    # we want to warn. But not below a given thresold.
+    _range_warning_thresold = 1024 * 1024
+
     def __init__(self):
         self._response = None
+        self._ranges_received_whole_file = None
 
     def _mutter_connect(self):
         netloc = self.host
@@ -183,7 +193,17 @@
         That makes the httplib.HTTPConnection happy
         """
         if self._response is not None:
-            self._response.finish()
+            pending = self._response.finish()
+            # Warn the user (once) that 
+            if (self._ranges_received_whole_file is None
+                and self._response.status == 200
+                and pending and pending > self._range_warning_thresold
+                ):
+                self._ranges_received_whole_file = True
+                trace.warning(
+                    'Got a 200 response when asking for multiple ranges,'
+                    ' does your server at %s:%s support range requests?',
+                    self.host, self.port)
             self._response = None
         # Preserve our preciousss
         sock = self.sock