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