----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63480/#review190175 -----------------------------------------------------------
Fix it, then Ship it! src/resource_provider/manager.cpp Lines 165 (patched) <https://reviews.apache.org/r/63480/#comment267442> There is no need to use an `Owned` here; a `ResourceProvider` would work just fine. The mutating `update*` functions should just take a `ResourceProvider*` in that case. They should do this even if we'd store an `Owned` here since they do not do anything with the passed provider's `Owned`'ness. src/resource_provider/manager.cpp Lines 348-349 (patched) <https://reviews.apache.org/r/63480/#comment267448> Does it make sense to just drop this message with some logging instead of failing over the agent? src/resource_provider/manager.cpp Line 337 (original), 413 (patched) <https://reviews.apache.org/r/63480/#comment267443> We should just take a `ResourceProvider*` here; these functions do not care about `resourceProvider`'s `Owned`'ness. src/resource_provider/manager.cpp Line 345 (original), 421 (patched) <https://reviews.apache.org/r/63480/#comment267444> We should just take a `ResourceProvider*` here; these functions do not care about `resourceProvider`'s `Owned`'ness. src/resource_provider/manager.cpp Lines 482 (patched) <https://reviews.apache.org/r/63480/#comment267445> Not sure if this function should be `const` as it maps a non-`const` process function. - Benjamin Bannier On Nov. 2, 2017, 12:14 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63480/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2017, 12:14 a.m.) > > > Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gaston Kleiman, > Greg Mann, and Jan Schlicht. > > > Repository: mesos > > > Description > ------- > > The resource provider manager provides an `applyOfferOperation` method > for offer operation affecting resource providers. The resources on > which the operation should be applied contains a resource provider ID. > This will be extracted and an event will be sent to the respective > resource provider. > > (This is based on https://reviews.apache.org/r/61810) > > > Diffs > ----- > > src/messages/messages.proto 9dfe7f07c2e1171be17f1bd7b6a54a80410c44f6 > src/resource_provider/manager.hpp 3b70e75c6b6721864ae0ee9c4a593b5035d8388f > src/resource_provider/manager.cpp 11f890156f0fd099f8a97b07cdc458a0726ee78e > src/slave/slave.cpp 817dca4fc3e1028ccc13b8a1bb1ee783afb77166 > > > Diff: https://reviews.apache.org/r/63480/diff/1/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
