BIND 10 #2922: enhance group subscriber management in msgq

[BIND 10 Development]

#2922: enhance group subscriber management in msgq
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  vorner
            Priority:  high          |                       Status:
           Component:  msgq          |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130625
         Sub-Project:  Core          |                   Resolution:
Estimated Difficulty:  4             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by pselkirk):

 * owner:  pselkirk => vorner


Comment:

 Replying to [comment:13 vorner]:
 > No, it sets up to 10 times the `wait_time`, so we wait at most
 `wait_time` seconds, which is 10 by default. This is the same as waiting
 for the whole cfgmgr to start up. But the action we wait for is much more
 lightweight (we don't wait for whole process to start up, just the startup
 of cfgmgr to bubble through the other thread of msgq). So it should pose
 no problems.

 Okay, thanks for clarifying that.

 > This is slightly more complex than you seem to think. The init in this
 snippet is actually a local variable holding the tested object, not the
 top-level module. So I had to import the ProcessStartError into the global
 scope, because init.ProcessStartError did _not_ refer to the class at that
 scope.
 >
 > If it is deemed too confusing, I probably could rename the init variable
 to something else.

 That is confusing, and unfortunately it's throughout the whole test
 module. The simplest thing at this point is probably to rename the local
 variable in the one test case to 'b10_init', which seems to be what other
 test cases do when they need to refer to the init module.

 > > What sort of trouble? Is this something we should be testing for?
 >
 > I added a description.

 Okay, thanks.

 > > I scratch my head about the usage of single vs double quotes.
 >
 > Actually, I just don't think much about the quotes. I have the habit
 from perl, where it is good to use both kinds, since each have different
 meaning. Sometimes in other languages which have the same meaning for both
 kinds, I tend to put machine-readable strings to single-quotes and human-
 readable ones to double ones, but as I don't think about them, I just mix
 them mostly randomly.

 I'm well aware of how perl uses single vs double quotes, and AIUI python
 does the same. And where the string is a literal that will be read the
 same with either kind of quotes, it comes down to matters of taste and
 convention. But when I see what appears to be inconsistency, I tend to
 wonder what the intent was. In any case, it's fine.

 > > `MsgQTest.test_notifies` should probably test unsubscription from a
 group not prevously subscribed to.
 >
 > OK

 So the result of this is that there will be no notifications from this
 'unsubscribe' command, because no state actually changed. That's a
 legitimate approach to take, and probably the safest thing to do when
 other users are listening to these notifications. (It's also been argued
 in similar contexts that the end result is that user is unsubscribed from
 this group, so it should return success.) In any case, I'll leave it to
 you. My main reason for bringing this up was that we should know what
 happens in this case.

 > > `test_notifies` and `test_notifies_implicit_kill` have some ugly code
 to "Steal the lname". Is there a reason you can't use `fd_to_lname`?
 >
 > The reason is I added the `fd_to_lname` later. I changed it, but it
 doesn't seem to be _much_ cleaner, just a tiny bit.

 Okay.

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