BIND 10 #2470: use MasterLoader in memory::ZoneDataLoader

[BIND 10 Development]

#2470: use MasterLoader in memory::ZoneDataLoader
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  data source   |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20121218
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  3             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  loadzone-ng
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Replying to [comment:15 vorner]:

 > > I can think of a couple of possibilities, but I'm afraid these are not
 > > a trivial task.  [...]
 >
 > I really dislike that approach ‒ as you mention, that beats the purpose
 of
 > separating the collator and clutters code.
 >
 > An option could be to add methods `reportError` and `reportWarning` to
 the
 > MasterLoader that would take only the text. The MasterLoader would add
 the
 > current file and line and call the callbacks.
 >
 > Then this method could be passed to the collator or whatever else could
 want to
 > keep checking things on the fly as they are being loaded (which includes
 any
 > possible future analyzers or other tools).

 That (kind of approach) was actually one of the possibilities I
 thought, but my personal conclusion was that it was not appealing
 either.  If we push the responsibility of this check to the user of
 the master loader, each user of the class needs to do the record
 keeping.  And, this case should be checked for the database-based
 loader, too, so we'll immediately repeat ourselves.  Also, when the
 `reportXXX` is called, printing the "current line" will be misleading,
 because the loader has already eaten the new line and is seeing the
 next line.  We could reduce the number by one as a heuristics measure,
 but that itself would be an ad hoc hack and obscure the
 understandability of the code (besides, technically, we cannot be sure
 if reducing the number is correct without knowing the reason for the
 message).  In addition to these the extension of the interface itself
 looks ad hoc to me; it just looks like we are introducing another
 complexity to the class as a result of hating a different type of
 complexity.

 I'm not necessarily saying this approach is worse than doing it within
 the loader, but at least to me it looks equally bad.

 > It won't. But it doesn't belong to the MasterLoader. As with the SOA,
 I'm
 > reluctant adding it there.
 >
 > > To this end, my suggestion is to remove the additional callback from
 > > the collator, and create a ticket to this extension.
 >
 > ACK, but I think we want to discuss the approach how it should be
 solved.
 >
 > But can you keep a LOG_WARN at least in the collator instead, for the
 meantime?
 > Because, seriously, I'm not sure the new ticket will be handled right
 away.

 You mean the collator calls logger directly?  If so, we cannot do
 this; libdns++ doesn't use logging directly, intentionally.

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