BIND 10 #3329: Integrate use of D2ClientMgr IO services into b10-dhcp4

[BIND 10 Development]

#3329: Integrate use of D2ClientMgr IO services into b10-dhcp4
-------------------------------------+-------------------------------------
            Reporter:  tmark         |                        Owner:  tmark
                Type:  enhancement   |                       Status:
            Priority:  medium        |  reviewing
           Component:  dhcp-ddns     |                    Milestone:  DHCP-
            Keywords:                |  Kea0.9-alpha
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DHCP          |                 CVSS Scoring:
Estimated Difficulty:  24            |              Defect Severity:  N/A
         Total Hours:  34            |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  2
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by marcin):

 * hours:  32 => 2
 * owner:  marcin => tmark
 * totalhours:  0 => 34


Comment:

 Reviewed commit e8513d7199db5ba79b1f91087f57eb54ab81ef74

 '''src/bin/dhcp4/ctrl_dhcp4_srv.cc'''
 establishSession: you should change the type of the exception being caught
 in this function so as it traps exceptions being thrown by startD2()
 function (at least function documentation says that it may throw
 !D2ClientError). Currently !DhcpConfigError is caught in establishSession.

 '''src/bin/dhcp4/dhcp4_messages.mes'''
 I think it would be better to refer the DHCP server as !''DHCPv4
 server!'', instead of !''IPv4 DHCP server!''.

 Also, spurious space before %1.

 '''src/bin/dhcp4/tests/d2_unittest.cc'''
 There is an unspecified todo in line 32.

 '''src/lib/dhcp_ddns/dhcp_ddns_messages.mes'''
 In this: !'' This is a programmatic error, highly unlikely to occur, and
 should not impair the application's to process requests.!'' - should it be
 !''application's ability!'', or so? Sounds like something is missing.

 '''src/lib/dhcp_ddns/ncr_io.h'''
 I don't understand why you have this warning: !''Running all ready
 handlers, in theory, could process all messages currently queued.!'' in
 the context of runReadyIO? This function is supposed to run one ready
 handler (not all). Even if I run it multiple times, what is the problem
 with processing all queued messages?

 '''src/lib/dhcp_ddns/tests/ncr_udp_unittests.cc'''
 The comment in line 492 is partly true as it doesn't check ready state via
 select but siumply calls ioReady:
 {{{
 // Make sure select_fd does evaluates to ready via select and
 // that ioReady() method agrees.
 ASSERT_TRUE(sender.ioReady());
 }}}

 Perhaps the comments should be modified to just say that we call ioReady.

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