----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69163/#review210993 -----------------------------------------------------------
src/resource_provider/manager.cpp Lines 579 (patched) <https://reviews.apache.org/r/69163/#comment295834> Did you intend to do this in the current patch or make this a TODO? src/tests/master_tests.cpp Lines 9226 (patched) <https://reviews.apache.org/r/69163/#comment295831> Could you run this test in repetition to ensure that we're not introducing any flakiness? src/tests/master_tests.cpp Lines 9271-9300 (patched) <https://reviews.apache.org/r/69163/#comment295830> This could be made more concise with the following pattern: ``` EXPECT_CALL(*scheduler, connected(_)) .WillOnce(v1::scheduler::SendSubscribe(v1::DEFAULT_FRAMEWORK_INFO)); Future<Event::Subscribed> subscribed; EXPECT_CALL(*scheduler, subscribed(_, _)) .WillOnce(FutureArg<1>(&subscribed)); ``` src/tests/master_tests.cpp Lines 9351-9352 (patched) <https://reviews.apache.org/r/69163/#comment295832> Seems like we might as well also verify that these fields contain the correct values? src/tests/mesos.hpp Lines 3165-3166 (patched) <https://reviews.apache.org/r/69163/#comment295833> Sorry I don't understand this comment - could you clarify? - Greg Mann On Nov. 30, 2018, 11:06 a.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69163/ > ----------------------------------------------------------- > > (Updated Nov. 30, 2018, 11:06 a.m.) > > > Review request for mesos, Chun-Hung Hsiao, Gastón Kleiman, and James DeFelice. > > > Bugs: MESOS-9293 > https://issues.apache.org/jira/browse/MESOS-9293 > > > Repository: mesos > > > Description > ------- > > This patch sets agent and/or resource provider ID operation status > update messages. This is not always possible, e.g., some operations > might fail validation so that no corresponding IDs can be extracted. > > Since operations failing validation are currently directly rejected by > the master without going through a status update manager, they are not > retried either. If a master status update manager for operations is > introduced at a later point it should be possible to forward > acknowledgements for updates to the master's update manager (not agent > ID, not resource provider ID). > > > Diffs > ----- > > src/common/protobuf_utils.hpp 1662125ed3e47b179ee32d08c1d3af75553066ba > src/common/protobuf_utils.cpp a45607eed4c4bae5010bcc3f3ffeabd6d911062a > src/master/master.cpp 3b3824a67f46866cd64e32d7f9f92484b5891aa2 > src/resource_provider/manager.cpp 6c81c430e9e1205d71982a7fa2bcd9aa15fc01b2 > src/resource_provider/storage/provider.cpp > a22c82c442304979fbdec0fcb74543077751a135 > src/slave/slave.cpp 858b78620e1ef33f3587d0bd95a684917aaf5bbb > src/tests/master_tests.cpp ef2c00101fc3d30c564a9ca34884dece2cdd2651 > src/tests/mesos.hpp c08e7e6c1dbc3dd9eb980868e43368c0a423c3ee > > > Diff: https://reviews.apache.org/r/69163/diff/5/ > > > Testing > ------- > > `make check` > > > Thanks, > > Benjamin Bannier > >
