> On Oct. 7, 2013, 11:25 p.m., Ben Mahler wrote:
> > src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp, lines 515-516
> > <https://reviews.apache.org/r/14292/diff/13/?file=361967#file361967line515>
> >
> >     Is there a plan for consolidating these two futures? Is there a 
> > specific version of mesos where we can merge them?

This has been changed in a subsequent review. please take a look there.


> On Oct. 7, 2013, 11:25 p.m., Ben Mahler wrote:
> > src/java/src/org/apache/mesos/MesosSchedulerDriver.java, line 175
> > <https://reviews.apache.org/r/14292/diff/13/?file=361968#file361968line175>
> >
> >     s/init/initializeWithCredentials/ ?
> >     
> >     Then maybe the comment isn't as necessary.

see above.


> On Oct. 7, 2013, 11:25 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 287-288
> > <https://reviews.apache.org/r/14292/diff/13/?file=361970#file361970line287>
> >
> >     It doesn't look like the master uses this? 
> >     
> >     Could we just create the vector on the stack in initialize()?

good catch. forgot to clean this up.


> On Oct. 7, 2013, 11:25 p.m., Ben Mahler wrote:
> > src/master/master.hpp, lines 295-298
> > <https://reviews.apache.org/r/14292/diff/13/?file=361970#file361970line295>
> >
> >     Leaving this here as a note from our discussion:
> >     
> >     Would it be cleaner to have a 'registering' map which stores the 
> > promises and then have a authenticated set which only contains 
> > authenticated pids?

decided to just use 'authenticating' map for this.


> On Oct. 7, 2013, 11:25 p.m., Ben Mahler wrote:
> > src/sched/sched.cpp, line 291
> > <https://reviews.apache.org/r/14292/diff/13/?file=361974#file361974line291>
> >
> >     Every argument you use in a bind or function object, and consequently 
> > in a dispatch or delay call, will be copied. So, I'm not sure this note is 
> > needed.
> >     
> >     Even if authenticationTimeout took a Future reference, it would be 
> > working on a copy of the future because the bind object copied it.

that makes sense. not sure what me/benh were worried about.


> On Oct. 7, 2013, 11:25 p.m., Ben Mahler wrote:
> > src/sched/sched.cpp, lines 354-356
> > <https://reviews.apache.org/r/14292/diff/13/?file=361974#file361974line354>
> >
> >     Even with:
> >     
> >     authenticationTimeout(Future<bool>& f)
> >     
> >     f would be a copy of the future when you called delay(), so I'm not 
> > sure this second note is needed?

fixed.


> On Oct. 7, 2013, 11:25 p.m., Ben Mahler wrote:
> > src/sched/sched.cpp, lines 891-897
> > <https://reviews.apache.org/r/14292/diff/13/?file=361974#file361974line891>
> >
> >     The use of the word "Flag" tripped me up here a bit in that I first 
> > thought that reauthenticate was a command-line flag.

rephrased.


- Vinod


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


On Oct. 7, 2013, 8:04 a.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14292/
> -----------------------------------------------------------
> 
> (Updated Oct. 7, 2013, 8:04 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Joe Smith, Kevin 
> Sweeney, Tobias Weingartner, and Bill Farner.
> 
> 
> Bugs: MESOS-704
>     https://issues.apache.org/jira/browse/MESOS-704
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added authentication support for scheduler driver and master.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto 957576bbc1c73513a9591194d017f76fe562a616 
>   include/mesos/scheduler.hpp cf3ecdaaf40fd878a80fe0b6f7e61a0997329cbd 
>   src/common/type_utils.hpp 674a8820c339c6446dfa7d444457477ab4512e79 
>   src/java/jni/construct.cpp b01bd7ae2eda2dc5e0dcd68848c65bd9f9ea81f0 
>   src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
> 6d2a03b6a88e71ac4e2e2d1ee8e15925e393ef3d 
>   src/java/src/org/apache/mesos/MesosSchedulerDriver.java 
> 7ef1fe7755286bf92b94d7ece4f72d54e5b57a84 
>   src/master/flags.hpp d59e67d5b2799d6d7a37e9cfe7246ae7372091ac 
>   src/master/master.hpp 0aeec7fc540d44c03c1171f31a7281a4b0055925 
>   src/master/master.cpp ce8365f082a5f96ef64e33e526cb5047dff52127 
>   src/python/native/mesos_scheduler_driver_impl.cpp 
> f25d41d38caf2701813dbec0d342a3b327e9dedf 
>   src/sasl/authenticator.hpp 2f78cf0fdd97f0ddc3a6ebd162e6559497d708e4 
>   src/sched/sched.cpp c399f2481259683a8e178abb3478307042292f23 
> 
> Diff: https://reviews.apache.org/r/14292/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Reply via email to