BIND 10 #1451: Create DDNS process module

BIND 10 Development do-not-reply at isc.org
Mon Dec 19 11:08:02 UTC 2011


#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:  9
-------------------------------------+-------------------------------------
Changes (by jelte):

 * owner:  jelte => jinmei


Comment:

 Replying to [comment:13 jinmei]:
 > It looks almost okay.  I have two last comments.
 >
 > - I'm afraid it could be dangerous to do all shutdown jobs in the
 >   middle of a signal handler.  For example, check_command() could
 >   still be called after shutdown(), but the latter may destruct some
 >   fundamental attribute of the class at that point.  In general, I
 >   think it's safer to minimize the things within a signal handler
 >   (e.g., just setting a flag) and do other complicated jobs in the
 >   usual control flow.  I have a proposed patch to implement this idea.
 >   (dddns.diff, attached)
 >

 Ah, now I get it; I was only looking at the code and didn't see the
 problem (as the proposed patch currently doesn't alter the behaviour). The
 comment 'and cleanup should go here' was incorrect, and probably the name
 too (these caused the confusion I think).

 Applied your patch, but with a few amendments; I think the logging of the
 event really should go in the trigger function (as the biggest reason to
 have two log messages is to be able to see if it didn't shut down
 immediately and why). And I renamed the former shutdown() to
 shutdown_cleanup(), for added clarity. Also added a line in docstring that
 trigger_shutdown() is the function one needs when the module needs to be
 stopped.

 This function is now a noop, which contradicts the no-unused-code
 principle, but as this whole dummy module is kind of a violation on that,
 I think it is ok.

 > - TestMain repeats the same pattern and I think it's better to combine
 >   them in a separate helper method.  See attached diff
 >   (exception.diff)
 >
 > If you are fine with these patched, please just apply them and merge.
 > If you simply don't agree, I'm okay with it for now; please feel free
 > to merge the current code.  If you don't agree right now but want to
 > discuss it more, please continue.

 Applied as-is.

 I assume the changes are trivial enough, but for completeness, I am
 assigning this back to you for one last time ;)

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


More information about the bind10-tickets mailing list