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