> On July 25, 2014, 2:15 p.m., Kenneth Giusti wrote: > > Looks good, with a few minor nits below. > > > > I think we should take a page from Python and only allow default arguments > > to come after non-default positional arguments. I don't think our current > > schema parser enforces this. I wouldn't make that change as part of this > > fix - maybe open a JIRA to address that later. Thoughts? > >
I agree that changing the schema parser shouldn't be part of this fix. I believe this fix will still work if/when the schema parser is changed. - Ernie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23897/#review48728 ----------------------------------------------------------- On July 24, 2014, 9:04 p.m., Ernie Allen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23897/ > ----------------------------------------------------------- > > (Updated July 24, 2014, 9:04 p.m.) > > > Review request for qpid, Justin Ross and Kenneth Giusti. > > > Repository: qpid > > > Description > ------- > > This patch attempts to handle the cases where the number of arguments > supplied to a method call don't match the number of arguments required by the > schema. > > When there are too many arguments supplied, the assumption is that the client > is aware of a new argument, but the broker isn't. This is currently the case > when qpid-route is dealing with an older broker and calls link.bridge(). The > console will look for a named argument that was supplied, but isn't in the > broker's schema. If one is found, it is removed. This requires that the new > argument be named when it is passed to console. Otherwise we would have to > assume the last argument supplied was the extra one, and that would be risky. > > When there are too few arguments supplied, the assumption is that client is > out of date and isn't aware a new argument that the schema requires. This is > currently the case when an old qpid-route is dealing with a new broker and > calls link.bridge(). The console will look for arguments in the schema that > have default values (starting at the end of the schema). If one is found, it > is added with the default value. This requires that all new arguments be > given a default value in the schema. > > > Diffs > ----- > > /trunk/qpid/extras/qmf/src/py/qmf/console.py 1613170 > /trunk/qpid/tools/src/py/qpid-route 1613170 > > Diff: https://reviews.apache.org/r/23897/diff/ > > > Testing > ------- > > schema: > <method name="test1"> > <arg name="a" dir="I" type="bool"/> > <arg name="b" dir="I" type="bool" default="True"/> > <arg name="c" dir="I" type="uint16"/> > <arg name="d" dir="I" type="uint32" default="2"/> > </method> > > client calls with console calls broker with > (False, False, 42, 1) a:False, b:False, c:42, d:1 > (False, 42) a:False, b:True, c:42, d:2 > (False, False, 42) a:False, b:False, c:42, d:2 > (False, 42, d=666) a:False, b:True, c:42, d:666 > (False, 42, b=False) a:False, b:False, c:42, d:2 > (False, 42, d=666, b=True) a:False, b:True, c:42, d:666 > (True, True, 42, 1, e="z") a:True, b:True, c:42, d:1 > (True, 42, e="z") a:True, b:True, c:42, d:2 > (False, 42, b=False, e="z") a:False, b:False, c:42, d:2 > > (False) Incorrect number of arguments: expected 4, > got 3 > (False, False, 42, 1, 0) Incorrect number of arguments: expected 4, > got 5 > (True, True, 42, 1, b=True) Incorrect number of arguments: expected 4, > got 5 > (True, e="z") Incorrect number of arguments: expected 4, > got 3 > > The following calls fail with "Failed: Exception - invalid conversion: > Cannot convert a (/home/eallen/22/qpid/cpp/src/qpid/types/Variant.cpp:132)" > (False, "a", d=666) a:False, b:True, c:"a", d:666 > (True, "a", 42) a:True, b:"a", c:42, d:2 > > As you can see from the last tests, I did not add any type checking to ensure > the supplied arguments match the schema before I make the actual call. > > > Thanks, > > Ernie Allen > >
