Patchwork D2859: wireproto: change and document results from reactor methods

login
register
mail settings
Submitter phabricator
Date March 14, 2018, 4:43 p.m.
Message ID <differential-rev-PHID-DREV-ga2d4bl3tlr2b2ksksh5-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/29503/
State Superseded
Headers show

Comments

phabricator - March 14, 2018, 4:43 p.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I think this makes the code a bit clearer. And documentation is
  always useful, right?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2859

AFFECTED FILES
  mercurial/wireprotoframing.py
  mercurial/wireprotoserver.py

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - March 22, 2018, 1:34 a.m.
durin42 added a comment.


  This one failed to apply. It kind of looks like it got folded into earlier revisions, is that right?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2859

To: indygreg, #hg-reviewers
Cc: durin42, mercurial-devel
phabricator - March 22, 2018, 1:36 a.m.
indygreg abandoned this revision.
indygreg added a comment.


  Yes. I folded this in to previous commits and `hg phabsend` wasn't smart enough to realize it.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2859

To: indygreg, #hg-reviewers
Cc: durin42, mercurial-devel

Patch

diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -397,8 +397,9 @@ 
         frametype, frameflags, payload = frame
         states.append(b'received: %d %d %s' % (frametype, frameflags, payload))
 
-        r = reactor.onframerecv(frametype, frameflags, payload)
-        states.append(json.dumps(r, sort_keys=True, separators=(', ', ': ')))
+        action, meta = reactor.onframerecv(frametype, frameflags, payload)
+        meta['action'] = action
+        states.append(json.dumps(meta, sort_keys=True, separators=(', ', ': ')))
 
     res.status = b'200 OK'
     res.headers[b'Content-Type'] = b'text/plain'
@@ -418,12 +419,7 @@ 
         if not frame:
             break
 
-        state = reactor.onframerecv(*frame)
-        if not state:
-            raise error.ProgrammingError(
-                'did not receive valid reply from server reactor')
-
-        action = state['action']
+        action, meta = reactor.onframerecv(*frame)
 
         if action == 'wantframe':
             # Need more data before we can do anything.
@@ -440,13 +436,13 @@ 
                 return
 
             _httpv2runcommand(ui, repo, req, res, authedperm, reqcommand,
-                              reactor, state)
+                              reactor, meta)
 
         elif action == 'error':
             # TODO define proper error mechanism.
             res.status = b'200 OK'
             res.headers[b'Content-Type'] = b'text/plain'
-            res.setbodybytes(state['message'] + b'\n')
+            res.setbodybytes(meta['message'] + b'\n')
             return
         else:
             raise error.ProgrammingError(
@@ -501,16 +497,16 @@ 
     res.headers[b'Content-Type'] = FRAMINGTYPE
 
     if isinstance(rsp, wireprototypes.bytesresponse):
-        action = reactor.onbytesresponseready(rsp.data)
+        action, meta = reactor.onbytesresponseready(rsp.data)
     else:
-        action = reactor.onapplicationerror(
+        action, meta = reactor.onapplicationerror(
             _('unhandled response type from wire proto command'))
 
-    if action['action'] == b'sendframes':
-        res.setbodygen(action['framegen'])
+    if action == 'sendframes':
+        res.setbodygen(meta['framegen'])
     else:
         raise error.ProgrammingError('unhandled event from reactor: %s' %
-                                     action['action'])
+                                     action)
 
 # Maps API name to metadata so custom API can be registered.
 API_HANDLERS = {
diff --git a/mercurial/wireprotoframing.py b/mercurial/wireprotoframing.py
--- a/mercurial/wireprotoframing.py
+++ b/mercurial/wireprotoframing.py
@@ -243,6 +243,32 @@ 
 
     I/O and CPU intensive operations are purposefully delegated outside of
     this class.
+
+    Consumers are expected to tell instances when events occur. They do so by
+    calling the various ``on*`` methods. These methods return a 2-tuple
+    describing any follow-up action(s) to take. The first element is the
+    name of an action to perform. The second is a data structure (usually
+    a dict) specific to that action that contains more information. e.g.
+    if the server wants to send frames back to the client, the data structure
+    will contain a reference to those frames.
+
+    Valid actions that consumers can be instructed to take are:
+
+    sendframes
+       Indicates that frames should be sent to the client. The ``framegen``
+       key contains a generator of frames that should be sent. The server
+       assumes that all frames are sent to the client.
+
+    error
+       Indicates that an error occurred. Consumer should probably abort.
+
+    runcommand
+       Indicates that the consumer should run a wire protocol command. Details
+       of the command to run are given in the data structure.
+
+    wantframe
+       Indicates that nothing of interest happened and the server is waiting on
+       more frames from the client before anything interesting can be done.
     """
 
     def __init__(self, ui, repo):
@@ -281,33 +307,29 @@ 
 
         The raw bytes response is passed as an argument.
         """
-        return {
-            'action': 'sendframes',
+        return 'sendframes', {
             'framegen': createbytesresponseframesfrombytes(data),
         }
 
     def onapplicationerror(self, msg):
-        return {
-            'action': 'sendframes',
+        return 'sendframes', {
             'framegen': createerrorframe(msg, application=True),
         }
 
     def _makeerrorresult(self, msg):
-        return {
-            'action': 'error',
+        return 'error', {
             'message': msg,
         }
 
     def _makeruncommandresult(self):
-        return {
-            'action': 'runcommand',
+        return 'runcommand', {
             'command': self._activecommand,
             'args': self._activeargs,
             'data': self._activedata.getvalue() if self._activedata else None,
         }
 
     def _makewantframeresult(self):
-        return {
+        return 'wantframe', {
             'action': 'wantframe',
             'state': self._state,
         }