----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69411/#review210956 -----------------------------------------------------------
I'll summarize a discussion we held offline, between BenM, Benno, and I. The immediate goal behind changing the interface for `cluster::Master` (and possibly `cluster::Slave`) is to remove the need for dozens of `StartMaster` and `StartSlave` overloads in the test helpers. Hopefully this change would also allow us to remove some extraneous lines scattered throughout the tests (like creating `Fetcher` objects for agents, even in tests that don't exercise the fetcher). Currently, we do not use "builder" interfaces that widely in the mesos codebase. Instead, we might want to use a struct instead. i.e. ``` MasterOptions options; options.flags.some_flag = non_default; Try<Owned<cluster::Master>> master = StartMaster(options); ``` The struct approach would need to deal with vague ownership of some objects, as both the struct and `Owned<cluster::Master>` would be accessible from within the test body. The `StartSlave` options struct may be more complicated than the master's, because there are several dependency chains in the agent. - Joseph Wu On Nov. 21, 2018, 5:21 p.m., Benno Evers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69411/ > ----------------------------------------------------------- > > (Updated Nov. 21, 2018, 5:21 p.m.) > > > Review request for mesos and Joseph Wu. > > > Repository: mesos > > > Description > ------- > > This review adds a new method `cluster::Master::create()` that > allows constructing masters while only specifying the non-standard > components required for the specific test, like this: > > auto master = cluster::Master::build() > .withZookeeperUrl(url) > .withAllocator(allocator); > > This is sometimes better known as "fluent interface". > > > Diffs > ----- > > src/tests/cluster.hpp ad2b80e658d2f8afcefe9969d62cd33f0c475ce9 > src/tests/cluster.cpp 2b351ca70d8e80008e49722aa7d46918b5ecd9b0 > > > Diff: https://reviews.apache.org/r/69411/diff/2/ > > > Testing > ------- > > > Thanks, > > Benno Evers > >
