[MERGE] Add call_with_body_stream to _SmartClient; add support for streamed request bodies to the server.

[John Arbash Meinel]

John Arbash Meinel has voted tweak.
Status is now: Conditionally approved
Comment:
+
+    Possible states:
+     * expecting args
+     * expecting body (terminated by receiving a post-body status)
+     * finished
      """


^- It seems that this is incomplete. I'm not sure what other states are
available, but at the very least I saw:

+            if byte == 'S':
+                # Success.  Nothing more to come except the end of 
message.
+                self.expecting = 'end'
+            elif byte == 'E':
+                # Error.  Expect an error structure.
+                self.expecting = 'error'

^- So one state is "expecting an error".

And:
+        self.expecting = 'nothing'
+        self.request_handler.end_received()

Certainly I can see how these would be "finished" but it seems 
reasonable to
specifically enumerate them.


I find it a little bit odd that the "accept_body" function is now 
calling
"do_chunk" rather than calling "do_body".


v- Having run into this elsewhere:
=== modified file 'bzrlib/smart/request.py'
--- bzrlib/smart/request.py     2008-10-30 03:47:59 +0000
+++ bzrlib/smart/request.py     2009-01-12 02:10:51 +0000
@@ -70,6 +70,7 @@
              if not root_client_path.endswith('/'):
                  root_client_path += '/'
          self._root_client_path = root_client_path
+        self._body_bytes = ''

      def _check_enabled(self):
          """Raises DisabledMethod if this method is disabled."""
@@ -110,11 +111,13 @@

          The do() method is still called, and must have returned None.
          """
-        raise NotImplementedError(self.do_chunk)
+        self._body_bytes += chunk_bytes

      def do_end(self):
          """Called when the end of the request has been received."""
-        pass
+        body_bytes = self._body_bytes
+        self._body_bytes = None
+        return self.do_body(body_bytes)

      def translate_client_path(self, client_path):
          """Translate a path received from a network client into a local
@@ -229,25 +232,17 @@
          self._backing_transport = backing_transport
          self._root_client_path = root_client_path
          self._commands = commands
-        self._body_bytes = ''
          self.response = None
          self.finished_reading = False
          self._command = None


^- It would be a lot better if "self._body_bytes" was a list that we 
appended
to, and then used ''.join() if we had to. Remember the terrible scaling 
of
'readv()' when it was reading 100MB, 64k chunks at a time.


=== modified file 'bzrlib/util/bencode.py'
--- bzrlib/util/bencode.py      2007-07-13 04:22:17 +0000
+++ bzrlib/util/bencode.py      2009-01-09 05:52:37 +0000
@@ -52,7 +52,7 @@
      while x[f] != 'e':
          v, f = decode_func[x[f]](x, f)
          r.append(v)
-    return (r, f + 1)
+    return (tuple(r), f + 1)


^- Also, these changes seem specifically incorrect now, as you don't 
really
want to universally change the api to return tuples instead of lists. It
probably just means that this can't land until some form of your other 
patch
lands.


For details, see: 
http://bundlebuggy.aaronbentley.com/project/bzr/request/%3C20090112044720.GA21968%40steerpike.home.puzzling.org%3E
Project: Bazaar