D2951: wireproto: use CBOR for command requests
This revision was automatically updated to reflect the committed changes.
Closed by commit rHG3d0e2cd86e05: wireproto: use CBOR for command requests
(authored by indygreg, committed by ).
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D2951?vs=7439=7553
REVISION DETAIL
https://phab.mercurial-scm.org/D2951
AFFECTED FILES
mercurial/help/internals/wireprotocol.txt
mercurial/wireprotoframing.py
mercurial/wireprotoserver.py
tests/test-http-api-httpv2.t
tests/test-wireproto-serverreactor.py
CHANGE DETAILS
diff --git a/tests/test-wireproto-serverreactor.py
b/tests/test-wireproto-serverreactor.py
--- a/tests/test-wireproto-serverreactor.py
+++ b/tests/test-wireproto-serverreactor.py
@@ -2,6 +2,9 @@
import unittest
+from mercurial.thirdparty import (
+cbor,
+)
from mercurial import (
util,
wireprotoframing as framing,
@@ -96,7 +99,8 @@
frames = list(framing.createcommandframes(stream, 1, b'command',
{}, data))
self.assertEqual(frames, [
-ffs(b'1 1 stream-begin command-name have-data command'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'command'}"),
ffs(b'1 1 0 command-data continuation %s' % data.getvalue()),
ffs(b'1 1 0 command-data eos ')
])
@@ -108,7 +112,8 @@
frames = list(framing.createcommandframes(stream, 1, b'command', {},
data))
self.assertEqual(frames, [
-ffs(b'1 1 stream-begin command-name have-data command'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'command'}"),
ffs(b'1 1 0 command-data continuation %s' % (
b'x' * framing.DEFAULT_MAX_FRAME_SIZE)),
ffs(b'1 1 0 command-data eos x'),
@@ -125,10 +130,9 @@
}, data))
self.assertEqual(frames, [
-ffs(b'1 1 stream-begin command-name have-args|have-data command'),
-ffs(br'1 1 0 command-argument 0 \x04\x00\x09\x00key1key1value'),
-ffs(br'1 1 0 command-argument 0 \x04\x00\x09\x00key2key2value'),
-ffs(br'1 1 0 command-argument eoa \x04\x00\x09\x00key3key3value'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'command', b'args': {b'key1': b'key1value', "
+b"b'key2': b'key2value', b'key3': b'key3value'}}"),
ffs(b'1 1 0 command-data eos %s' % data.getvalue()),
])
@@ -286,10 +290,9 @@
stream = framing.stream(1)
results = list(sendcommandframes(reactor, stream, 41, b'mycommand',
{b'foo': b'bar'}))
-self.assertEqual(len(results), 2)
-self.assertaction(results[0], 'wantframe')
-self.assertaction(results[1], 'runcommand')
-self.assertEqual(results[1][1], {
+self.assertEqual(len(results), 1)
+self.assertaction(results[0], 'runcommand')
+self.assertEqual(results[0][1], {
'requestid': 41,
'command': b'mycommand',
'args': {b'foo': b'bar'},
@@ -301,11 +304,9 @@
stream = framing.stream(1)
results = list(sendcommandframes(reactor, stream, 1, b'mycommand',
{b'foo': b'bar', b'biz': b'baz'}))
-self.assertEqual(len(results), 3)
-self.assertaction(results[0], 'wantframe')
-self.assertaction(results[1], 'wantframe')
-self.assertaction(results[2], 'runcommand')
-self.assertEqual(results[2][1], {
+self.assertEqual(len(results), 1)
+self.assertaction(results[0], 'runcommand')
+self.assertEqual(results[0][1], {
'requestid': 1,
'command': b'mycommand',
'args': {b'foo': b'bar', b'biz': b'baz'},
@@ -329,7 +330,8 @@
def testmultipledataframes(self):
frames = [
-ffs(b'1 1 stream-begin command-name have-data mycommand'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'mycommand'}"),
ffs(b'1 1 0 command-data continuation data1'),
ffs(b'1 1 0 command-data continuation data2'),
ffs(b'1 1 0 command-data eos data3'),
@@ -350,9 +352,9 @@
def testargumentanddata(self):
frames = [
-ffs(b'1 1 stream-begin command-name have-args|have-data command'),
-ffs(br'1 1 0 command-argument 0 \x03\x00\x03\x00keyval'),
-ffs(br'1 1 0 command-argument eoa \x03\x00\x03\x00foobar'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'command', b'args': {b'key': b'val',"
+b"b'foo': b'bar'}}"),
ffs(b'1 1 0 command-data continuation value1'),
ffs(b'1 1
D2951: wireproto: use CBOR for command requests
indygreg updated this revision to Diff 7439.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D2951?vs=7328=7439
REVISION DETAIL
https://phab.mercurial-scm.org/D2951
AFFECTED FILES
mercurial/help/internals/wireprotocol.txt
mercurial/wireprotoframing.py
mercurial/wireprotoserver.py
tests/test-http-api-httpv2.t
tests/test-wireproto-serverreactor.py
CHANGE DETAILS
diff --git a/tests/test-wireproto-serverreactor.py
b/tests/test-wireproto-serverreactor.py
--- a/tests/test-wireproto-serverreactor.py
+++ b/tests/test-wireproto-serverreactor.py
@@ -2,6 +2,9 @@
import unittest
+from mercurial.thirdparty import (
+cbor,
+)
from mercurial import (
util,
wireprotoframing as framing,
@@ -96,7 +99,8 @@
frames = list(framing.createcommandframes(stream, 1, b'command',
{}, data))
self.assertEqual(frames, [
-ffs(b'1 1 stream-begin command-name have-data command'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'command'}"),
ffs(b'1 1 0 command-data continuation %s' % data.getvalue()),
ffs(b'1 1 0 command-data eos ')
])
@@ -108,7 +112,8 @@
frames = list(framing.createcommandframes(stream, 1, b'command', {},
data))
self.assertEqual(frames, [
-ffs(b'1 1 stream-begin command-name have-data command'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'command'}"),
ffs(b'1 1 0 command-data continuation %s' % (
b'x' * framing.DEFAULT_MAX_FRAME_SIZE)),
ffs(b'1 1 0 command-data eos x'),
@@ -125,10 +130,9 @@
}, data))
self.assertEqual(frames, [
-ffs(b'1 1 stream-begin command-name have-args|have-data command'),
-ffs(br'1 1 0 command-argument 0 \x04\x00\x09\x00key1key1value'),
-ffs(br'1 1 0 command-argument 0 \x04\x00\x09\x00key2key2value'),
-ffs(br'1 1 0 command-argument eoa \x04\x00\x09\x00key3key3value'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'command', b'args': {b'key1': b'key1value', "
+b"b'key2': b'key2value', b'key3': b'key3value'}}"),
ffs(b'1 1 0 command-data eos %s' % data.getvalue()),
])
@@ -286,10 +290,9 @@
stream = framing.stream(1)
results = list(sendcommandframes(reactor, stream, 41, b'mycommand',
{b'foo': b'bar'}))
-self.assertEqual(len(results), 2)
-self.assertaction(results[0], 'wantframe')
-self.assertaction(results[1], 'runcommand')
-self.assertEqual(results[1][1], {
+self.assertEqual(len(results), 1)
+self.assertaction(results[0], 'runcommand')
+self.assertEqual(results[0][1], {
'requestid': 41,
'command': b'mycommand',
'args': {b'foo': b'bar'},
@@ -301,11 +304,9 @@
stream = framing.stream(1)
results = list(sendcommandframes(reactor, stream, 1, b'mycommand',
{b'foo': b'bar', b'biz': b'baz'}))
-self.assertEqual(len(results), 3)
-self.assertaction(results[0], 'wantframe')
-self.assertaction(results[1], 'wantframe')
-self.assertaction(results[2], 'runcommand')
-self.assertEqual(results[2][1], {
+self.assertEqual(len(results), 1)
+self.assertaction(results[0], 'runcommand')
+self.assertEqual(results[0][1], {
'requestid': 1,
'command': b'mycommand',
'args': {b'foo': b'bar', b'biz': b'baz'},
@@ -329,7 +330,8 @@
def testmultipledataframes(self):
frames = [
-ffs(b'1 1 stream-begin command-name have-data mycommand'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'mycommand'}"),
ffs(b'1 1 0 command-data continuation data1'),
ffs(b'1 1 0 command-data continuation data2'),
ffs(b'1 1 0 command-data eos data3'),
@@ -350,9 +352,9 @@
def testargumentanddata(self):
frames = [
-ffs(b'1 1 stream-begin command-name have-args|have-data command'),
-ffs(br'1 1 0 command-argument 0 \x03\x00\x03\x00keyval'),
-ffs(br'1 1 0 command-argument eoa \x03\x00\x03\x00foobar'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'command', b'args': {b'key': b'val',"
+b"b'foo': b'bar'}}"),
ffs(b'1 1 0 command-data continuation value1'),
ffs(b'1 1 0 command-data eos value2'),
]
@@ -371,76 +373,68 @@
'data': b'value1value2',
})
-def
D2951: wireproto: use CBOR for command requests
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
Now that we're using CBOR in the new wire protocol, let's convert
command requests to it.
Before I wrote this patch and was even thinking about CBOR, I was
thinking about how commands should be issued and came to the
conclusion that we didn't need separate frames to represent the
command name from its arguments. I already had a partially
completed patch prepared to merge the frames.
But with CBOR, it makes the implementation a bit simpler because
we don't need to roll our own serialization.
The changes here are a bit invasive. I tried to split this into
multiple commits to make it easier to review. But it was just too
hard.
- "command name" and "command argument" frames have been collapsed into a
"command request" frame.
- The flags for this new frame are totally different.
- Frame processing has been overhauled to reflect the new order of things.
- Test fallout was significant. A handful of tests were removed.
Altogether, I think the new code is simpler. We don't have
complicated state around receiving commands. We're either receiving
command request frames or command data frames. We /could/
potentially collapse command data frames into command request
frames. Although I'd have to think a bit more about this before
I do it.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2951
AFFECTED FILES
mercurial/help/internals/wireprotocol.txt
mercurial/wireprotoframing.py
mercurial/wireprotoserver.py
tests/test-http-api-httpv2.t
tests/test-wireproto-serverreactor.py
CHANGE DETAILS
diff --git a/tests/test-wireproto-serverreactor.py
b/tests/test-wireproto-serverreactor.py
--- a/tests/test-wireproto-serverreactor.py
+++ b/tests/test-wireproto-serverreactor.py
@@ -2,6 +2,9 @@
import unittest
+from mercurial.thirdparty import (
+cbor,
+)
from mercurial import (
util,
wireprotoframing as framing,
@@ -96,7 +99,8 @@
frames = list(framing.createcommandframes(stream, 1, b'command',
{}, data))
self.assertEqual(frames, [
-ffs(b'1 1 stream-begin command-name have-data command'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'command'}"),
ffs(b'1 1 0 command-data continuation %s' % data.getvalue()),
ffs(b'1 1 0 command-data eos ')
])
@@ -108,7 +112,8 @@
frames = list(framing.createcommandframes(stream, 1, b'command', {},
data))
self.assertEqual(frames, [
-ffs(b'1 1 stream-begin command-name have-data command'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'command'}"),
ffs(b'1 1 0 command-data continuation %s' % (
b'x' * framing.DEFAULT_MAX_FRAME_SIZE)),
ffs(b'1 1 0 command-data eos x'),
@@ -125,10 +130,9 @@
}, data))
self.assertEqual(frames, [
-ffs(b'1 1 stream-begin command-name have-args|have-data command'),
-ffs(br'1 1 0 command-argument 0 \x04\x00\x09\x00key1key1value'),
-ffs(br'1 1 0 command-argument 0 \x04\x00\x09\x00key2key2value'),
-ffs(br'1 1 0 command-argument eoa \x04\x00\x09\x00key3key3value'),
+ffs(b'1 1 stream-begin command-request new|have-data '
+b"cbor:{b'name': b'command', b'args': {b'key1': b'key1value', "
+b"b'key2': b'key2value', b'key3': b'key3value'}}"),
ffs(b'1 1 0 command-data eos %s' % data.getvalue()),
])
@@ -286,10 +290,9 @@
stream = framing.stream(1)
results = list(sendcommandframes(reactor, stream, 41, b'mycommand',
{b'foo': b'bar'}))
-self.assertEqual(len(results), 2)
-self.assertaction(results[0], 'wantframe')
-self.assertaction(results[1], 'runcommand')
-self.assertEqual(results[1][1], {
+self.assertEqual(len(results), 1)
+self.assertaction(results[0], 'runcommand')
+self.assertEqual(results[0][1], {
'requestid': 41,
'command': b'mycommand',
'args': {b'foo': b'bar'},
@@ -301,11 +304,9 @@
stream = framing.stream(1)
results = list(sendcommandframes(reactor, stream, 1, b'mycommand',
{b'foo': b'bar', b'biz': b'baz'}))
-self.assertEqual(len(results), 3)
-self.assertaction(results[0], 'wantframe')
-self.assertaction(results[1], 'wantframe')
-self.assertaction(results[2], 'runcommand')
-self.assertEqual(results[2][1], {
+self.assertEqual(len(results), 1)
+self.assertaction(results[0], 'runcommand')
+
