BIND 10 #1596: Addressing of multiple instances of the same component

[BIND 10 Development]

#1596: Addressing of multiple instances of the same component
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  vorner                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20120221
                  Component:         |            Resolution:
  Unclassified                       |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  5
Feature Depending on Ticket:         |           Total Hours:  0
  Multiple auths                     |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:9 jinmei]:
 > > That is to be expected. This is solved in #1595, which is already
 merged in master, but not in this branch yet. If you merge with master
 locally, you should see that it works (or you could trust me I tried it
 O:-).
 >
 > Ah, okay, I trust you:-) but I wish you could have noted it on asking
 > the review.

 Sorry, I didn't think about it, it seemed unrelated to the ticket.

 > > Shouldn't I regenerate it when I change it? (it's always a problem
 when we have a generated file in the repository). Anyway, I wanted to the
 output, so I needed to regenerate it. Should I revert the html back?
 >
 > As someone who reviews the branch, it's more convenient if the diff
 > only contains essential changes.  It would also help reduce conflicts
 > on merge if different developers modify the guide in different
 > branches.  I think this is something we should discuss at the project
 > level (a candidate biweekly call item?).  For this particular branch
 > I'd leave it to you whether to revert it.

 OK, I'll try not to commit it next time because of the review, but I think
 it makes no sense to revert it now (and produce another large commit
 without any essential changes). The problem with conflicts is small one
 and easy to address, another regeneration solves it.

 > - auth_log.h: not a big deal (and not really specific to this ticket),
 >   but I wonder why we define local log levels like this:
 > {{{#!c++
 > // Debug messages upon shutdown
 > const int DBG_AUTH_SHUT = DBGLVL_START_SHUT;
 > }}}
 >   especially now that we have use this value for two purposes.  Why
 >   can't we simply use DBGLVL_START_SHUT for both cases?

 I think for historical reasons, we didn't have the DBGLVL_START_SHUT at
 the time when most of the logging was written and when they appeared, the
 local constants were just updated. I didn't want to break the consistency
 with them and I don't want to go through the whole auth and modify them
 there, the cleanup change would be too big to piggy-back it I think. Maybe
 another ticket (the tickets seem to grow exponentially this way, every
 ticket has multiple follow-up tickets).

 > - AuthCommand::exec: in general, I'd rather prefer rejecting bogus
 >   inputs (either as a result of exception or by returning a specific
 >   error) than ignoring them.  In the case of auth, it should actually
 >   have been so because Element class would throw an exception (and it
 >   will be caught in execAuthServerCommand(), logged, and translated
 >   into an error answer to the command issuer).  Same comment applies
 >   to the resolver, although in that case we (probably) cannot simply
 >   throw an exception because it wouldn't be caught in a proper
 >   context.

 I changed it to throw the exception (and added the catch for the resolver,
 it can't hurt).

 Thank you

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