> On Dec. 23, 2014, 8:56 a.m., Niklas Nielsen wrote: > > src/watcher/whitelist_watcher.cpp, lines 60-61 > > <https://reviews.apache.org/r/28514/diff/2/?file=780056#file780056line60> > > > > Can you explain _why_ you only need to call the subscriber with an > > empty whitelist when an initial list was provided (not only the fact that > > it's happening). Does that mean that if the user doesn't specify a list, > > the subscriber won't be notified? I have a hard time understanding how this > > fixed the gmock issue and remains correct. > > Alexander Rukletsov wrote: > The expected logic (codified in tests and leading to gmock issues) is to > notify the subscriber once and only if a change to the previous state occurs. > If we do not want to use whitelist ("*" is passed) and the initial whitelist > was not provided (`intialWhitelist == None()`), the subscriber should not be > notified, because no change to the previous state occurs. However, if the > initial whitelist is, for example, set to empty (`intialWhitelist == {}`) for > security reasons, then passing "*" should trigger notification. > > Regarding you question, if the user does not specify a list > (`intialWhitelist` defaults to `None()`), the subscriber won't be notified > only if "*" is passed. > > Does it makes sense?
> if the user does not specify a list (intialWhitelist defaults to None()), the > subscriber won't be notified only if "*" is passed. Initial whitelist is (when this patch lands) always None() and that is maybe the thing that I have a hard time understanding. You have pointed me to a dependent review which starts using it. Can you mark it as dependent on this one? The comment is confusing to me as the code below doesn't use the initial list but is generalized to 'lastWhitelist' (which makes more sense to me). The explanation I have been looking for is why the initial case needs to be dealt with in a special way. When would you set a initial whitelist which has not been read from a file by the whitelist watcher? Also, how can 'lastWhitelist' ever be Some() when path is '*'? Does the patch effectively just remove the 'subscriber(None());' line? Thanks for helping me understand this. - Niklas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28514/#review65954 ----------------------------------------------------------- On Dec. 2, 2014, 3:10 a.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28514/ > ----------------------------------------------------------- > > (Updated Dec. 2, 2014, 3:10 a.m.) > > > Review request for mesos, Cody Maloney, Niklas Nielsen, and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > The subscriber may now provide an initial whitelist and will be notified only > when the parsed whitelist differs from the initial one. The subscriber is not > explicitly notified that there is no whitelist unless they have provided a > valid initial whitelist before. This change suppresses gmock warnings for > uninteresting mock function calls. > > > Diffs > ----- > > src/watcher/whitelist_watcher.hpp 5838854 > src/watcher/whitelist_watcher.cpp 32713bb > > Diff: https://reviews.apache.org/r/28514/diff/ > > > Testing > ------- > > make check (Mac OS 10.9.4, Ubuntu 14.04) > checked test log for gmock warnings. > > > Thanks, > > Alexander Rukletsov > >
