BIND 10 #2954: DHCP-DDNS: Create BIND 10 process skeleton

[BIND 10 Development]

#2954: DHCP-DDNS: Create BIND 10 process skeleton
-------------------------------------+-------------------------------------
            Reporter:  stephen       |                        Owner:
                Type:  enhancement   |  marcin
            Priority:  medium        |                       Status:
           Component:  dhcp          |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-DHCP-20130523
         Sub-Project:  DHCP          |                   Resolution:
Estimated Difficulty:  0             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by tmark):

 * owner:  tmark => marcin


Comment:

 Replying to [comment:4 marcin]:
 > Reviewed commit e448bbba3ecae68b261612954aa9777edc384be4
 >
 > I have made this comment some time ago via email. I believe that in
 D2Srv, the !''Srv!'' is redundant or even wrong. Please note that none of
 the modules in BIND10 are called like this. For example, b10-dhcp4 is
 indeed a server process but it is called b10-dhcp4. Perhaps, simply using
 d2 would be sufficient. If we are not concerned about the length of the
 name of this module, we could even try !''dhcpddns!''. Even in this stub
 implementation, there is a lot of !''Srv!'' in the code already.
 Programmers are lazy by their nature, so writing !''Srv!'' everywhere
 might be a pain for some people :-).
 >

 After discussing the issue with the the Dhcp team consensus to remove,
 "Srv" and "srv" throughout.  Attempting to work in dhcp-ddns or
 permutations thereof is cumbersome at best.  The module will be "b10-d2"
 and files renamed as follows:

 bind10/src/bin/d2
     Makefile.am
     b10-d2.xml
     d2.spec
     d2_log.cc
     d2_log.h
     d2_messages.mes
     main.cc
     spec_config.h.pre.in

     tests/
         Makefile.am
         d2_test.py
         d2_unittests.cc


 > '''b10-d2srv.xml'''
 > Copyright date should be corrected in file header.
 >
 > Errors in the following tags:
 > - refentryinfo - invalid date
 > - refnamediv - refpurpose is invalid - copied from DHCP module
 > - docinfo - invalid date
 > - cmdsynopsis - the (-v) option is not supported right now, so there is
 no reason to document it.
 > - refsynopsisdiv - same as above with -v option
 >

 Corrected.


 > '''d2srv_log.cc'''
 > Invalid copyright date.
 >
 > Comment before !''include!'' does not match the module. We define the
 logger for !''d2!'', not b10-dhcp4.
 >
 > '''d2srv_log.h'''
 > Copyright date is invalid.
 > Comment before !''extern!'' is copy-pasted from DHCP4 and thus invalid
 in this context.
 >
 > '''d2srv_messages.mes'''
 > Invalid copyright date.
 >


 Corrected.

 > D2SRV_STARTING & D2SRV_START_INFO: Shouldn't it be !''D2Controller!'',
 instead of !''DController!''?
 >

 Corrected.

 > D2SRV_START_INFO: Do we really plan to have the ability to run the
 server in a standalone mode? If we do, how do we get the configuration?
 > BTW, if we decide that we need it (for example to run unit tests), it
 should be documented in b10-d2srv.xml?

 I based this on dhcp4/6.  Since they allegedly support "verbose" and
 "stand-alone" flags, I presumed D2 should as well.  It was my
 understanding that we wanted D2 to be capbale of running without bind10.
 I have been working under the assumption that the arguments claimed in
 dhcp4 and dhcp6 function as advertised. Do they not?

 I added a comment stating that the parameters are preliminary.

 >
 > '''main.cc'''
 > Invalid copyright date.
 >

 Corrected.

 > usage(): what is the [-p number] parameter? It is not described
 anywhere, including the error message printed by the usage(). In fact, any
 attempt to specify this parameter would result in error and usage
 printing.
 >

 Removed reference to -p.

 > Shouldn't verbose mode be only available when standalone is turned on?
 We control the verbosity by other means, i.e. using bindctl if running a
 module using bind10.

 See my comment above regarding standalone mode.

 >
 > main(): Why define the !''ret!'' variable? You could do:
 > {{{
 > return (EXIT_SUCCESS);
 > }}}

 This was residual from the dchp4 version main.cc, which was more involved
 than this main is at the moment. I have cleaned it up a bit.

 >
 > '''Makefile.am'''
 > Do we really have to link with all these libraries already? I would be
 in favor to link against the minimal number of libraries and extend the
 dependencies as we go. The libb10-dhcp++.la is already linked with the
 number of libs, so I don't know whether explicitly adding these is really
 needed.
 >

 Again, residual



 > '''spec_config.h.pre'''
 > Copyright date is invalid.
 >
 > The location of the spec file is also invalid.
 >
 > '''tests/Makefile.am'''
 > The same comment regarding linking with many libraries as above.
 >
 > '''tests/d2srv_unittests.cc'''
 > Copyright date in the header is invalid.
 >
 > '''tests/d2srv_test.py'''
 > Copyright date is invalid.
 >
 > Do we actually have a ticket to implement !''test_alive!''? Currently
 the comment says it is just a placeholder.
 >

 I made the test functional.

 > '''!ChangeLog'''
 > If we wanted to follow the convention from the !ChangeLog we would
 rather say:
 > {{{
 > 6xx. [func] tmark
 >     b10-d2srv: Initial DHCP-DDNS (a.k.a. D2) module implemented.
 Currently it does
 >     nothing useful, except for providing the skeleton implementation
 >     to be expanded in the future.
 >     (Trac #2954, git TBD)
 > }}}
 >

 This wording is fine.

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