BIND 10 #1789: update xfrin to maintain in-memory/sqlite3 zones

[BIND 10 Development]

#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      |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:9 jelte]:

 > > - This is rather a question about the general configuration framework,
 > >   but I wonder what's the assumption on the validity of given data on
 > >   the remote config.  [...]
 >
 > Yes, the whole point of the specification file to validate data was that
 once the data gets to the 'handler' routines, they won't have to worry
 about the data being bad (at least in its form and structure)
 >
 > One thing that I am considering for the api changes in config use is
 more validation (probably automatic; implicit in the initialization of the
 classes), so that it will indeed be validated each and every time the data
 is initialized in every module. Most of that may be redundant but it
 should never be on a critical performance path anyway. Especially since I
 also want to start using 'changes' somehow; instead of passing full lists,
 pass the elements that are added or removed (memory zones are a good
 example of why we need that; if you have 10000 zones you do not want to
 reload the entire list if you add or remove one ;))

 Revising the interface that way is probably good.  But note that
 there'll be some semantics errors that would be beyond the scope of
 checks by the config module.  At least unless we introduce the notion
 of user-defined "types" such as domain names, we'd not be able to
 validate a string that is supposed to be a valid domain name within
 the config module; it can only check if it's a valid "string".
 There'll be even more complicated semantics errors such as non
 existence of a file or a prohibited combination of some options, etc.

 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.

 Comments on the revised code follow:

 First, I made a couple of editorial fixes.

 '''xfrin.py'''

 - technically, there are still some exception guarantee issues for
   `_auth_config_handler`, such as the case where `_set_db_file`
   succeeds but `_set_memory_zones` doesn't.  But it's probably too
   minor and these configurations will be short term workaround
   (datasource-related config should be eventually more generalized),
   so I'm okay with leaving it as it is.

 - 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.

 - textual representation of RR classes is also case sensitive.
   Should we also normalize (and test) 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.

 - 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.

 - 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"
 }}}

 '''xfrin_test.py'''

 - Are these multiple blank lines (instead of one) intentional?
 {{{#!python
         self.assertTrue(self.xfr._is_memory_zone("example.com", "CH"))




 def raise_interrupt():
 }}}

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