> On April 23, 2018, 11:30 p.m., Chun-Hung Hsiao wrote: > > src/resource_provider/manager.cpp > > Lines 685 (patched) > > <https://reviews.apache.org/r/66545/diff/6/?file=2009017#file2009017line685> > > > > Let's move the actual logic from r/66546 to this patch to make it > > self-contained. > > > > Also, what's your opinion on having a different error message for > > re-subscribing with an ID that has been removed?
I don't believe merging this patch with r/66546 is a good idea as they have different goals. The patch here is also self-cointained in that it compiles and passes all tests. Dropping. We don't currently (here or in r/66546) handle removed RP IDs at all. There also is currently no way to remove an RP as the needed operator API calls do not exist, yet. Let's discuss what to do about error messages when we implement that part. > On April 23, 2018, 11:30 p.m., Chun-Hung Hsiao wrote: > > src/resource_provider/manager.cpp > > Lines 688 (patched) > > <https://reviews.apache.org/r/66545/diff/6/?file=2009017#file2009017line688> > > > > It's probably just my personal preference, but based on the experience > > working on the codebase, i'd prefer `then` over `onAny` because the former > > gives the caller more control on error handling, rather than relying on the > > callee checking states. > > > > Please feel free to drop this if you think `onAny` is a better > > construct here. I agree slighlty :D While `then` leads to nicer code on the happy path, I feel it requires a lot more code to handle all the different failure scenarios then a simple check in an `onAny` handler (at least when using `void` continutations). This is what I choose an `onAny` here. Dropping. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66545/#review201766 ----------------------------------------------------------- On April 23, 2018, 1:19 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66545/ > ----------------------------------------------------------- > > (Updated April 23, 2018, 1:19 p.m.) > > > Review request for mesos, Chun-Hung Hsiao, Greg Mann, and Jan Schlicht. > > > Bugs: MESOS-8402 > https://issues.apache.org/jira/browse/MESOS-8402 > > > Repository: mesos > > > Description > ------- > > Added admitted resource providers to the manager's registry. > > > Diffs > ----- > > src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a > src/resource_provider/registrar.cpp > 92ef9aecb1e93d10f46e53984391558f9901a60b > src/resource_provider/registry.proto > 14bd433ef056f8891e9a38153fdb06c39cee8f62 > src/tests/resource_provider_manager_tests.cpp > c52541bf130ccf4795b989b53331176a64a097ea > > > Diff: https://reviews.apache.org/r/66545/diff/7/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
