BIND 10 #1789: update xfrin to maintain in-memory/sqlite3 zones
BIND 10 Development
do-not-reply at isc.org
Thu Apr 12 10:52:53 UTC 2012
#1789: update xfrin to maintain in-memory/sqlite3 zones
-------------------------------------+-------------------------------------
Reporter: | Owner: jinmei
jinmei | Status: reviewing
Type: task | Milestone:
Priority: | Sprint-20120417
medium | Resolution:
Component: xfrin | Sensitive: 0
Keywords: | Sub-Project: DNS
Defect Severity: N/A | Estimated Difficulty: 3
Feature Depending on Ticket: xfr | Total Hours: 0
for in-memory |
Add Hours to Ticket: 0 |
Internal?: 0 |
-------------------------------------+-------------------------------------
Changes (by jelte):
* owner: jelte => jinmei
Comment:
Replying to [comment:10 jinmei]:
> Replying to [comment:9 jelte]:
>
> So, regardless of whether and how we improve the config interface, I
> suspect we'll still have to think about when/where to validate remote
> configs.
>
ack, but let's discuss more of that outside of a ticket that will be
closed at some point :)
> - Likewise, the following part has a slight chance of causing exception:
> {{{#!python
> self._clear_memory_zones()
> for zone_str, class_str in new_memory_zones:
> self._add_memory_zone(zone_str, class_str)
> }}}
> and could be replaced with a complete swapping of the sets:
> {{{#!python
> self._memory_zones = new_memory_zones
> }}}
> but I'd leave it to you.
>
also a bit faster, though that shouldn't matter much here. Originally i
wanted to keep the details of the set clean from this method but now that
you mention this, it's probably unnecessary. Changed.
> - textual representation of RR classes is also case sensitive.
> Should we also normalize (and test) it?
>
it was already normalized, updated tests for it
> - Another note, which I didn't notice in the first review: depending
> on how we actually use it in #1790, we may have to protect the
> access to `_memory_zones` from multiple threads. IMO the current
> way of using threads in xfrin only makes it less readable and bug
> prone, but fixing it is a completely different topic. At least
> within this ticket there's no such issue, but you may want to leave
> a comment about this point somewhere in the code so we can remember
> it in #1790.
>
ok, added comments in the two 'main' method docstrings
> - What does 'eee' stand for?
> {{{#!python
> except Exception as eee:
> }}}
> (since it's not referenced anyway, maybe we could make it "unnamed",
> i.e. `_`). Not a big deal, anyway. I'd leave it to you.
>
leftover, removed
> - I'd note this hack should be gone in a longer term:
> {{{#!python
> if is_default and "B10_FROM_BUILD" in os.environ:
> # override the local database setting if it is default and
we
> # are running from the source tree
> db_file = os.environ["B10_FROM_BUILD"] + os.sep +\
> "bind10_zones.sqlite3"
> }}}
> like this:
> {{{#!python
> if is_default and "B10_FROM_BUILD" in os.environ:
> # override the local database setting if it is default and
we
> # are running from the source tree (For a longer term it
> # should be hidden inside the data source library and/or
> # done as a configuration, and this special case should be
gone).
> db_file = os.environ["B10_FROM_BUILD"] + os.sep +\
> "bind10_zones.sqlite3"
> }}}
>
ok
> '''xfrin_test.py'''
>
> - Are these multiple blank lines (instead of one) intentional?
> {{{#!python
> self.assertTrue(self.xfr._is_memory_zone("example.com", "CH"))
>
no, removed
--
Ticket URL: <http://bind10.isc.org/ticket/1789#comment:12>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list