BIND 10 #1512: implement zone section processing in DDNS

BIND 10 Development do-not-reply at isc.org
Wed May 23 10:08:44 UTC 2012


#1512: implement zone section processing in DDNS
-------------------------------------+-------------------------------------
                   Reporter:  jelte  |                 Owner:  jinmei
                       Type:  task   |                Status:  reviewing
                   Priority:         |             Milestone:
  medium                             |  Sprint-20120529
                  Component:  DDNS   |            Resolution:
                   Keywords:         |             Sensitive:  0
            Defect Severity:  N/A    |           Sub-Project:  DNS
Feature Depending on Ticket:  DDNS   |  Estimated Difficulty:  6
        Add Hours to Ticket:  0      |           Total Hours:  0
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:12 jinmei]:
 > > The `test_hash` ‒ as the hashes returned by it are deterministic, we
 could find a pair of names that return different values and check it.
 >
 > Hmm, I'm not sure if we can safely assume that - it depends on the
 > underlying boost hash implementation.  But I've added a case for
 > different hashes based on my local environment anyway.  If it turns
 > out we were naive via buildbot or other developer's reports, we can
 > then disable it.

 Ah, I see. I thought we just implemented it ourself.

 > > What is the motivation to have classes for formatting data
 (`ZoneFormatter` and `ClientFormatter`)? If it is performance, is it
 really faster to allocate the new python object than to do the actual
 conversion?
 > But you might think any of these are just premature.  In that case I'm
 > also okay with always converting them to string.

 It is probably premature, but as it is already written this way, I think
 it is OK to just add a comment there, explaining, and be done with it.

 > They are, but when the test fails the output is less informative.  If
 > we change the code to:
 >
 > {{{#!python
 >         #self.assertEqual(Opcode.UPDATE().to_text(),
 msg.get_opcode().to_text())
 >         self.assertEqual(Opcode.NOTIFY(), msg.get_opcode())
 > }}}
 >
 > we'll see:
 >
 > {{{
 > AssertionError: <pydnspp.Opcode object at 0x1018bbd50> !=
 <pydnspp.Opcode object at 0x1018bbd10>
 > }}}
 >
 > On the other hand, I admit it's counter intuitive.  I've added a
 > simple comment about the intent, but I'm also okay with removing
 to_text().

 A comment is good for now. I think the Opcode, and others, should have
 working __str__ method in the long term, so we wouldn't need these kinds
 of hacks, but it's out of the scope here. If you agree it would be good,
 would you create a ticket?

 > Perhaps the best way is to add some interface to just know the
 > existence of a zone with some abstract form of "zone ID" when found,
 > which the caller can subsequently use as a shortcut for more specific
 > actions (get updater, finder, iterator, etc).
 >
 > That will be far beyond of this branch, and for this branch I don't
 > have a strong opinion.  On one hand, the overhead of doing the search
 > twice wouldn't be of much concern for our initial implementation; but
 > that also applies to the overhead of starting an update transaction
 > possibly too early.
 >
 > If you have a specific suggestion, I'm okay with adopting it.

 No, I don't have anything specific in mind. Considering this is python, we
 probably don't need to worry about it too much anyway, I just wanted to
 point it out in case you would have a simple idea how to avoid it.

 So, except for the comment above, I think it is OK to merge. Would you add
 it and merge, if you agree?

 Thank you

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


More information about the bind10-tickets mailing list