mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alexander Rukletsov" <a...@mesosphere.io>
Subject Re: Review Request 28514: Updated WhitelistWatcher behaviour and introduced initial whitelist.
Date Tue, 02 Dec 2014 11:08:35 GMT


> On Dec. 1, 2014, 4:10 p.m., Niklas Nielsen wrote:
> > src/watcher/whitelist_watcher.cpp, line 64
> > <https://reviews.apache.org/r/28514/diff/1/?file=777491#file777491line64>
> >
> >     Can you help me understand this statement? Why do you need a variable to test
lastWhitelist against?
> 
> Alexander Rukletsov wrote:
>     Because I my call `subscriber()` later on.
> 
> Niklas Nielsen wrote:
>     // If no whitelist file is given, no need to watch. Notify the
>       // subscriber that there is no whitelist only if a valid initial
>       // whitelist has been provided.
>       if (path == "*") { // Accept all nodes.
>         VLOG(1) << "No whitelist given";
>         if (lastWhitelist.isSome()) {
>           subscriber(None());
>         }
>       } else {
>         watch();
>       }
>     
>     should work too, no? (Compiles fine)

Excellent change.


> On Dec. 1, 2014, 4:10 p.m., Niklas Nielsen wrote:
> > src/watcher/whitelist_watcher.hpp, line 44
> > <https://reviews.apache.org/r/28514/diff/1/?file=777490#file777490line44>
> >
> >     I am not sure I understand the reason behind initialWhitelist - why do we need
to treat the first pass of the whitelist differently?
> 
> Alexander Rukletsov wrote:
>     WhitlistWatcher calls the subscriber only if the whitelist changes. If you start
from a custom whitelist, you would like to pass it to the watcher.
> 
> Niklas Nielsen wrote:
>     That logic is hard to tell from the naming :-(
>     Maybe the whitelist variable is overloaded or at least requires some more documentation.

I will add a comment.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28514/#review63374
-----------------------------------------------------------


On Nov. 27, 2014, 12:33 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28514/
> -----------------------------------------------------------
> 
> (Updated Nov. 27, 2014, 12:33 p.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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message