BIND 10 #1258: "preserve order" option for Message::fromWire()

BIND 10 Development do-not-reply at isc.org
Fri Sep 30 18:29:52 UTC 2011


#1258: "preserve order" option for Message::fromWire()
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20111011
  blocker                            |            Resolution:
                  Component:         |             Sensitive:  0
  libdns++                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  0
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:5 jelte]:

 Thanks for the prompt review.

 > I've pushed a trivial comment change in message.h (and my editor removed
 a few
 > spaces in that file)

 Looks good, thanks.

 > Code looks good; just one small comment:
 >
 > message_python.cc:688: Shouldn't we just let the normal python error be
 > propagated here? It should also specify which types are wanted, albeit
 only
 > from the second call. Otherwise, I'd propose to at least add which types
 we
 > expect to the error message (just 'invalid arguments' is true but not
 extremely
 > helpful).

 Hmm, I added explicit PyErr_SetString() just following the convention
 adopted in many other such cases of our binding without considering
 the implications much.  So I checked the behavior without removing
 PyErr_SetString(), and I found it possibly more confusing in some cases.
 For example, from_wire(1, 1) would result in:
 {{{
 TypeError: function takes exactly 1 argument (2 given)
 }}}
 This is rather confusing as this method should actually accept 2
 arguments.

 On the other hand, I agree that just saying 'invalid arguments' isn't
 helpful either.

 So, I chose to keep our own PyErr_SetString() and tried to make the
 error string more helpful.  Please check.

-- 
Ticket URL: <http://bind10.isc.org/ticket/1258#comment:6>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list