[bind10-dev] proposal: separate ModuleCCSession and ConfigData

[Michal 'vorner' Vaner]

Hello

I like the general idea of splitting them. But I'd have few comments.

On Thu, Mar 29, 2012 at 12:28:51AM -0700, JINMEI Tatuya / 神明達哉 wrote:
> It's more so as we see the need for newer concepts such as logging
> related configuration or modules that don't belong to a process (so
> there's no spec file), like "tsig_key".  On the other hand,

Just a small clarification, even such modules do have a spec file. It's just
using a well-known name of the module is easier than having to dig through
directories and try to find it when it is available from the cfgmgr anyway.


About the new concepts, I'd like to have another one: notifications. A module
could broadcast an event and anyone would be allowed to subscribe to it.

Also, I'd like to see a simple way to address separate processes, not just the
current groups of them.

Another thing (which is more protocol problem than of the classes), when
returning an error, it is hardcoded to be [rcode, "error string"] ‒ I saw some
code restricting it. I'd like to be able to pass any data with the error (I
actually had to restrict some information once already when sending the error
back, as it would not get through).

>     // Adds the module name to the internal subscription list with the
>     // callback, and if not yet, subscribe to new messages for the module.
>     // When a non config command is received via the session, it extracts
>     // the data from the message, and calls the callback with it.
>     // callback is supposed to return a null ElementPtr if it's not expected
>     // respond to the update (as in the case of subscribing to changes to
>     // other modules).
>     void setCommandCallback(const std::string& module_name,
>                             CallbackType callback);

I'd actually prefer to have something like „Add callback for a command of this
name“, not a one-callback-fits-all. This way it would be more modular and we
could avoid handling things like parameter validation and unknown command
handling.


Also, I don't think we should distinguish the „our config data“ from „foreign
config data“ too much. If we would have the ConfigData class, I think the
ModuleCCSession should inherit from it, because handling own config data should
contain all the things as handling any config data (merging the configurations,
etc). It would just need some more code for validation, etc.

With regards

-- 
The problem with graduate students, in general, is that they have
to sleep every few days.

Michal 'vorner' Vaner
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <https://lists.isc.org/pipermail/bind10-dev/attachments/20120402/d8bfa4ea/attachment-0001.bin>