BIND 10 #1451: Create DDNS process module

[BIND 10 Development]

#1451: Create DDNS process module
-------------------------------------+-------------------------------------
                   Reporter:  jelte  |                 Owner:  jinmei
                       Type:  task   |                Status:  reviewing
                   Priority:  major  |             Milestone:
                  Component:  DDNS   |  Sprint-20111220
                   Keywords:         |            Resolution:
            Defect Severity:  N/A    |             Sensitive:  0
Feature Depending on Ticket:  DDNS   |           Sub-Project:  DNS
        Add Hours to Ticket:  0      |  Estimated Difficulty:  6
                  Internal?:  0      |           Total Hours:  6
-------------------------------------+-------------------------------------

Comment (by jinmei):

 '''ddns.py.in'''

 - general: I think we try to provide docstring for all
   classes/functions/methods by default (some existing python
   apps/modules are bad examples in this sense as many of them were
   implemented at an earlier stage of development and often skipped
   docs/tests with an excuse of lack of time)

 - general: personally, I'm not convinced the use of threads for our
   Python servers is a good design.  In particular, xfrout results in a
   very complicated mixture of (nest of!) threads and asynchronous I/O,
   and kills a major advantage of thread-style: straightforward code
   logic.  The current snippet of ddns makes me afraid about it going
   to be a second version of xfrout spaghetti.  Since DNS transactions
   are generally quite simple in terms of communication style
   (basically a transaction consists of a sequence of one request and
   one response, and especially in the case of UDP there shouldn't be
   any blocking operation other than for receiving requests), my gut
   feeling is that a simple form of event-style design with an
   asynchronous I/O primitive is sufficient.  Maybe I'm wrong, but at
   least I'd like to leave this point open until we really implement
   this part.

 - general: While looking at the ddns code, I had a feeling that we are
   now duplicating the same code logic for several Python programs in
   initialization and main command/configuration loop, and wondered
   whether we might extract the common pattern (maybe with some
   generalization) into a shared module.  This may be beyond the scope
   of this ticket, and we may rather complete the ddns work faster with
   possibly duplicate code, but I thought it's at least worth
   considering.

 - config_handler: in this simple form I don't see the need for the
   'answer' variable, but maybe it's introduced with expectation of
   extending the method (in a way that would require an intermediate
   variable like this) pretty soon.  Same for command_handler.

 - signal_handler: I suspect sys.exit should be outside of the if
   block.

 - signal_handler: is it a good idea to directly call shutdown() and
   exit here?  Assuming we would normally receive the signal in the
   main loop, may be it's better to set some flag to indicate the loop
   should be terminated?  That way we could unify the shutdown sequence
   both by a signal and by a command.  Also, I guess we should disable
   the signal handler (so we ignore these signals) once we receive it;
   otherwise shutdown() could be called in the middle of it.

 - main: ddns_server should be declared as global; otherwise it would
   be always None in signal_handler (note also that the fact that we
   cannot this in tests would indicate we'll need to think about how to
   test code here.)  But, more fundamentally, I would personally avoid
   relying on such a wider scope mutable variable.  Couldn't it be more
   sophisticated, e.g, by using a closure?

 - does it make sense to do anything specific for -v (--verbose)?
   we should probably still accept it because the bind10 process
   may specify it, but shouldn't we simply ignore it (or perhaps
   dump a message saying it's obsoleted)?

 '''ddns_test.py'''

 - It would be better to provide as many tests as possible.  For
   example, we should be able to test DDNSServer.config_handler and
   command_handler.

 '''man page'''
 - this may be obvious, but I think we should explain what "DDNS"
   stands for somewhere in documentation.  In the current
 - do we need to cleanup this?  Or complete it?
 {{{
        TODO TODO
 }}}
 - In the synopsis:
 {{{
        b10-ddns [-v] [--verbose]
 }}}
  (I'd rather hide the existence of this option if my understanding of
 these options is correct, but that aside) shouldn't this be
 `[-v|--verbose]`?
 - is it intentional that "zones" is not documented (for
   configuration)?

 '''Others'''
 - I made some trivial (I believe) editorial fixes/cleanups.
 - Do we need changelog?  too early?

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