> On July 2, 2018, 10:42 p.m., Benjamin Mahler wrote: > > src/master/allocator/mesos/hierarchical.cpp > > Line 1756 (original), 1755 (patched) > > <https://reviews.apache.org/r/67777/diff/2/?file=2047194#file2047194line1756> > > > > How about a member function of the framework struct? > > > > ``` > > if (!framework.isCapableOnAgent(slave)) { > > ... > > } > > ``` > > Meng Zhu wrote: > That would need to move nested Slave class forward or outside. Currently, > all the helper functions for slave/framework are member functions of the > outer class. I was trying to be consistent. If you think it is necessary, I > can go ahead and overhaul the structure. > > Meng Zhu wrote: > I could forward declare Slave in the outer class. Yet it would still > depend on `filterGpuResources` of the outer class. Need to either cache a > reference or pass in the flag. Feel cleaner this way.
Which functions are you referring to? I only see isRemoteSlave and isWhitelisted that look like they should be members of Slave? Yeah I think the ideal would be to pull these structs out of being nested so that the logic can be more clearly divided between them and the allocator, like we did in master.hpp for Slave and Framework structs. We can just put the TODOs in this patch and follow up separately for that, can you mark as fixed by adding TODOs? - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67777/#review205647 ----------------------------------------------------------- On July 4, 2018, 12:19 a.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67777/ > ----------------------------------------------------------- > > (Updated July 4, 2018, 12:19 a.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-8916 > https://issues.apache.org/jira/browse/MESOS-8916 > > > Repository: mesos > > > Description > ------- > > `isFrameworkCapableReceivingAgent` checks if a framework > is capable of receiving resources on the agent based on > the framework capability. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 > src/master/allocator/mesos/hierarchical.cpp > 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b > > > Diff: https://reviews.apache.org/r/67777/diff/3/ > > > Testing > ------- > > make check > > > Thanks, > > Meng Zhu > >
