BIND 10 #2908: Python wrapper for ZoneTableAccessor and getZoneTableAccessor

BIND 10 Development do-not-reply at isc.org
Mon Jun 10 08:25:26 UTC 2013


#2908: Python wrapper for ZoneTableAccessor and getZoneTableAccessor
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  pselkirk
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130611
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => pselkirk


Comment:

 Hello

 First, may I suggest that you make smaller commits next time? There were a
 lot of unrelated things changes in this one. It is easy to show bunch of
 commits as one diff if they are too small, but the reverse is almost
 impossible.

 Replying to [comment:11 pselkirk]:
 > > If the python methods are named `get_zone_table` and `get_zones`, the
 C++ wrapper functions should be renamed accordingly.
 >
 > I renamed the python methods to match the C++ methods.

 That wasn't what I was suggesting. I don't have strong opinion on that,
 but what I originally thought was to not stop the rename
 `95af51c86a9d480562526421863e47df7260e6a3` halfway through, but complete
 it with renaming functions like
 `ConfigurableClientList_getZoneTableAccessor` too.

 Anyway, I'll leave this up to you.

 What use is the get_current method now? Isn't it enough to keep the
 function as internal helper and not show it? Can you show a bit of python
 code where the method would be of any use?

 Also, the point with not needing get_iterator() was not addressed (or I
 didn't notice). Is it on purpose or did you overlook it? Also, if it's on
 purpose, can you share the reason for not agreeing on that?

 And, I pushed a very small fix of whitespace. Also, we usually write this
 test:

 {{{#!python
 self.assertEqual(origin.to_text(), "example.org.")
 }}}

 in the reverse form:
 {{{#!python
 self.assertEqual(origin, isc.dns.Name("example.org"))
 }}}

 As it takes care of all the details how to properly compare names. It is
 true we know that to_text() should return the canonical form, but still,
 that's one more detail to rely on during the test.

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


More information about the bind10-tickets mailing list