----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60283/#review178746 -----------------------------------------------------------
Fix it, then Ship it! src/master/master.cpp Line 4222 (original), 4222 (patched) <https://reviews.apache.org/r/60283/#comment252914> remove "after validation" src/master/master.cpp Lines 4760 (patched) <https://reviews.apache.org/r/60283/#comment252915> Per discussion, would be good to consider whether to validate/upgrade a task and its resources in a single operation. src/tests/master_tests.cpp Lines 7130 (patched) <https://reviews.apache.org/r/60283/#comment252916> `using` for `google::protobuf::RepeatedPtrField` ? src/tests/master_tests.cpp Lines 7132 (patched) <https://reviews.apache.org/r/60283/#comment252917> Can we add a comment here? ``` // If reservation refinement is enabled, inbound // resources should already be in the "post-refinement" format and should not need to be upgraded. ``` src/tests/master_tests.cpp Lines 7143 (patched) <https://reviews.apache.org/r/60283/#comment252924> Comment would be helpful here also. src/tests/master_tests.cpp Lines 7201 (patched) <https://reviews.apache.org/r/60283/#comment252918> Can we remove the `url` stuff? src/tests/master_tests.cpp Lines 7249 (patched) <https://reviews.apache.org/r/60283/#comment252919> Update or remove this comment. src/tests/master_tests.cpp Lines 7403 (patched) <https://reviews.apache.org/r/60283/#comment252921> "a RESERVE" src/tests/master_tests.cpp Lines 7406 (patched) <https://reviews.apache.org/r/60283/#comment252920> "an UNRESERVE" src/tests/master_tests.cpp Lines 7407 (patched) <https://reviews.apache.org/r/60283/#comment252922> "we test" - Neil Conway On June 21, 2017, 7:08 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60283/ > ----------------------------------------------------------- > > (Updated June 21, 2017, 7:08 p.m.) > > > Review request for mesos and Neil Conway. > > > Repository: mesos > > > Description > ------- > > Initially, it seemed like calling `convertResourceFormat` after > operation validation seemed safe since the operation validation > themselves performed `Resources::validate` within them. > > However, the rest of the operation validation code relies on > the fact that the resources have been validated, and uses > functions such as `isDynamicallyReserved`. Since functions such as > `isDynamicallyReserved` now requires "post-reservation-refinement" > format, we must perform this conversion earlier. > > In this patch, we use `upgradeResources` to perform resources > validation __and__ convert the resources before going into the > operation and task validation. > > We really need a better plan for this going forward. MESOS-7702. > > > Diffs > ----- > > src/master/master.cpp ec594a8f4fa95e77fc38103c5561d1797fe2b133 > src/tests/master_tests.cpp 652f37ab6f640ac87c48e419680501b705e11394 > src/tests/resource_offers_tests.cpp > c2bbf834c1d46079af492887b9dd40e57f3f2ac7 > > > Diff: https://reviews.apache.org/r/60283/diff/8/ > > > Testing > ------- > > > Thanks, > > Michael Park > >
