[bind10-dev] [sprint planning] estimate results 20111206

[JINMEI Tatuya / 神明達哉]

At Sat, 26 Nov 2011 11:33:38 +0100,
Michal 'vorner' Vaner <michal.vaner at nic.cz> wrote:

> > > > - - for #1395, the question was whether this wasn't a feature, not a bug
> > > > - - for #1398, the question was whether this is needed and the code isn't
> > > > already quite readable
> > 
> > #1398 is not a bug, but a suggested refactoring.
> 
> It is. However, I think that the current code is relatively easy to read. Maybe
> just if we had a specific variable saying which type of stream it is. But having
> a subclass for each type of scream means we will have the iteration through the
> stream many times and they are mostly identical (as there's already a
> polymorphism on the datasrc level) and there are only slight exceptions, like
> the SOA handling. I believe introducing another level of indirection just
> increases the size of code and makes it harder to grasp what is happening in
> this case.

I personally believe that the current style of if-else analysis tends
to cause subtle bugs in boundary conditions and still personally
believe it's worth generalizing.  But at the same time I admit that
this may be a matter of preference and that the current code is not
particularly unreadable.  So, in the sense of avoiding
over/premature-generalization I'm okay with not doing this task.

> > I believe #1395 is a bug to be fixed (someday).
> 
> I remember a discussion about just this. That if any logging is used without it
> being initialized, we must throw an exception, because it is possible someone
> forgot and it would be bad.
> 
> And the datasrc library does log its own actions (which is reasonable to do).
> 
> So from this, I deduce that these two facts already lead to the fact the library
> is _required_ to throw if it is used and logging is not initialized.
> 
> I don't think we want to rip the logging out. Neither I think we would like to
> wrap all logging statements in some kind of „if (log_enable)“ or
> „#ifdef LOG_ENABLE“. And setting log_enable = false is same amount of work to
> call log_init("something", LOG_NONE) (+- exact syntax).
> 
> Therefore, the question to ask here is, do we want to revisit the previous
> decision to throw on uninitialized use of logging? Is it better to be safe or to
> save users the one line of initializing logging? (and even if we didn't throw,
> the library still depends on the liblog, there are at last the calls to
> is_XXXX_enabled checks).

Hmm, okay.  I agree we should explicitly signal it if someone lazy
among us forgets necessary logging setup rather than silently hiding
it.  But I also think it's important to keep our public libraries
easier to use for external developers, and I still think the fact that
they need to write the following when they don't need logging is quite
annoying.

import isc.log
...
isc.log.init()

At the very least, the traceback isn't helpful as a hint to solve the
problem (*I* know many details of the BIND 10 system and could figure
what was missing from the output, but *they* wouldn't).

So I'd keep this ticket as a task to find a reasonable middleground.

---
JINMEI, Tatuya
Internet Systems Consortium, Inc.