> On March 17, 2014, 7:57 p.m., Dominic Hamon wrote: > > include/mesos/mesos.proto, line 497 > > <https://reviews.apache.org/r/18730/diff/4/?file=520216#file520216line497> > > > > why isn't this just a repeated string?
This is because, if we use a repeating string, there is no way to differentiate someone setting 'ips' to an empty list (denying everyone access) to someone not setting ips at all (access is determined by 'permissive') :( I would love to avoid it if I could. Let me know if there is a way? > On March 17, 2014, 7:57 p.m., Dominic Hamon wrote: > > src/authorizer/authorizer.hpp, line 84 > > <https://reviews.apache.org/r/18730/diff/4/?file=520218#file520218line84> > > > > explicit Is this needed for constructors that take Protobufs? I'm curious how would one make a mistake here? > On March 17, 2014, 7:57 p.m., Dominic Hamon wrote: > > src/authorizer/authorizer.hpp, line 89 > > <https://reviews.apache.org/r/18730/diff/4/?file=520218#file520218line89> > > > > does this need to be in the header? If so, does the implementation of > > the constructor need to be inline? I kept it in the header for simplicity (similar to what we did with authenticator and authenticatee). A method defined inside the class is implicitly inlined. > On March 17, 2014, 7:57 p.m., Dominic Hamon wrote: > > src/authorizer/authorizer.hpp, line 92 > > <https://reviews.apache.org/r/18730/diff/4/?file=520218#file520218line92> > > > > explicit See above. > On March 17, 2014, 7:57 p.m., Dominic Hamon wrote: > > src/authorizer/authorizer.hpp, line 100 > > <https://reviews.apache.org/r/18730/diff/4/?file=520218#file520218line100> > > > > this is heavily tied to the protocol buffer structure. Is there any way > > you can use reflection to build this cache? > > But the Role/User ACLs are a bit different from the URL ACL. I'm not sure using reflection here would make the code more readable here? > On March 17, 2014, 7:57 p.m., Dominic Hamon wrote: > > src/authorizer/authorizer.hpp, line 420 > > <https://reviews.apache.org/r/18730/diff/4/?file=520218#file520218line420> > > > > do you think it's worth validating in the Process when you're building > > the hashmaps, or have you split this into two phase on purpose? The reason to do the validation outside of the Process is to signify the clients to gracefully act on bad ACLs. If we were to do it in the Process, there's not much we can do except an EXIT. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18730/#review37442 ----------------------------------------------------------- On March 15, 2014, 12:45 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18730/ > ----------------------------------------------------------- > > (Updated March 15, 2014, 12:45 a.m.) > > > Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen. > > > Bugs: MESOS-911 > https://issues.apache.org/jira/browse/MESOS-911 > > > Repository: mesos-git > > > Description > ------- > > See summary. > > > Diffs > ----- > > include/mesos/mesos.proto 37f8a7fcd23d467b1274c46c405b836510afbd49 > src/Makefile.am ce7913be8432290a01fdcdede0fb9b5233745031 > src/authorizer/authorizer.hpp PRE-CREATION > src/tests/authorization_tests.cpp PRE-CREATION > src/tests/master_contender_detector_tests.cpp > 8da7420e18c7a960b566fae13a5975857eb777ee > > Diff: https://reviews.apache.org/r/18730/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
