BIND 10 #565: Boss should start/stop b10-auth and b10-recurse without restart

BIND 10 Development do-not-reply at isc.org
Wed Feb 23 18:58:51 UTC 2011


#565: Boss should start/stop b10-auth and b10-recurse without restart
-------------------------------------+-------------------------------------
                 Reporter:  vorner   |                Owner:  jinmei
                     Type:  defect   |               Status:  reviewing
                 Priority:  major    |            Milestone:  A-Team-
                Component:  Boss of  |  Sprint-20110309
  BIND                               |           Resolution:
                 Keywords:           |            Sensitive:  0
Estimated Number of Hours:  3.0      |  Add Hours to Ticket:  0
                Billable?:  1        |          Total Hours:  0
                Internal?:  0        |
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:7 vorner]:

 > >  - In StartAllProcessesBob, we probably don't need all of the stop_xxx
 > >    (e.g., stop_ccsession())
 >
 > I want to have them in case someone starts using stop_ something that is
 not used now and forgets to update the tests. This way, all possible ones
 are overriden.

 OK.

 > I should have addressed everything mentioned in the comments.

  - As for config_handler() refactoring, the idea looks good, but it's not
 readable to me.  It took some time for me to understand the calls to
 start_stop() is not an indentation error.  It's also not easy to
 understand the entire logic of the main function.  I'd at least like to
 see the main part in a unified place:
 {{{
         if self.verbose:
             sys.stdout.write("[bind10] Handling new configuration: " +
                 str(new_config) + "\n")
         # no intermediate large def here.
         start_stop('resolver', self.started_resolver_family, resolver_on,
             resolver_off)
         # or here
         start_stop('auth', self.started_auth_family, auth_on, auth_off)

         answer = isc.config.ccsession.create_answer(0)
         return answer
         # TODO
 }}}
   BTW, I just noticed the "TODO" comment doesn't make sense (understanding
 it's a leftover problem before this patch).  If there's a specific TODO
 item, we should describe it; if it was now garbage, we should remove it.
  - As for s/second/sendto/ in stop_process(), there was a clear typo.
 I've fixed it.  You didn't say anything about the naming, so I was not
 sure if you disagreed with me or overlooked that part of comment, but in
 any case this is a minor point and if that's intentional I'm okay with
 keeping the name.
  - As for renaming StartAllProcessesBob, I realized the comment was
 unclear, sorry.  I mainly intended to point out that it's now not specific
 to "start", so it would better be something like "StartStopCheckBob"
  - In the following, s/should running/should be running/?  Also, not all
 of them are "processes".
 {{{
         Check that the right sets of processes are started. The ones that
         should running are specified by the core, auth and resolver
 parameters
         (they are groups of processes, eg. auth means b10-auth, -xfrout,
 -xfrin
         and -zonemgr).
 }}}

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


More information about the bind10-tickets mailing list