> On June 26, 2019, 1:21 p.m., Benjamin Mahler wrote: > > src/master/quota.hpp > > Line 64 (original), 64 (patched) > > <https://reviews.apache.org/r/70951/diff/2/?file=2152271#file2152271line64> > > > > What is this comment saying? This constructor is for the UPDATE_QUOTA > > master call path? Why say that? > > > > There's also no other constructor, and this comment seems to suggest > > that there are others for other `XXX_QUOTA` things?
ah, left over from previous iteration. Removed. > On June 26, 2019, 1:21 p.m., Benjamin Mahler wrote: > > src/tests/registrar_tests.cpp > > Lines 944-946 (original), 965-967 (patched) > > <https://reviews.apache.org/r/70951/diff/2/?file=2152274#file2152274line977> > > > > Shouldn't we be sanity checking the registry protobuf at this point? > > This can be done by hitting the registrar's endpoint. > > > > Ditto in the other cases. We could do that. But I think checking the recovered state does that indirectly and that is consistent with other tests. Let's keep it as is? > On June 26, 2019, 1:21 p.m., Benjamin Mahler wrote: > > src/tests/registrar_tests.cpp > > Lines 954-960 (original), 975-986 (patched) > > <https://reviews.apache.org/r/70951/diff/2/?file=2152274#file2152274line987> > > > > I'm finding this test rather challenging to read, can we make it more > > readable than all the micro-checks by showing clearly what the expected > > registry looks like in json? E.g. > > > > ``` > > Registry expected = fromJSON(R"~( > > { > > "configs": [ ... ] > > "capabilities": [ ... ] > > } > > )~"); > > > > EXPECT_EQ(expected, actual); > > ``` > > > > That will make it a lot less of a struggle to read Used message differencer to avoid comparing field by field. - Meng ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70951/#review216159 ----------------------------------------------------------- On June 26, 2019, 9:50 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70951/ > ----------------------------------------------------------- > > (Updated June 26, 2019, 9:50 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-9601 > https://issues.apache.org/jira/browse/MESOS-9601 > > > Repository: mesos > > > Description > ------- > > The new operations will mutate the `quota_configs` field in > the registry to persist `QuotaConfigs` configured by the new > `UPDATE_QUOTA` call as well as the legacy `SET_QUOTA` and > `REMOVE_QUOTA` calls. > > The operation removes any entries in the legacy `quotas` field > with the same role name. In addition, it also adds/removes the > minimum capability `QUOTA_V2` accordingly: if `quota_configs` > is empty the capability will be removed otherwise it will > be added. > > This operation replaces the `REMOVE_QUOTA` operation. > > Also fixed affected tests. > > > Diffs > ----- > > src/master/master.cpp 826362d1a9ed034a49934b1810d6e1e6cffd90cd > src/master/quota.hpp 959e312b861fc0b59519a9ab5cc51061b55b534c > src/master/quota.cpp a7ee845d5916e859218b0168c7b682f77549aafc > src/master/quota_handler.cpp 476e87eb67e07a2ec6c7b258667cf94ff9c4f8eb > src/tests/registrar_tests.cpp 81979d78f1348791e28f1afea4ca2999de6362b8 > > > Diff: https://reviews.apache.org/r/70951/diff/3/ > > > Testing > ------- > > make check > > > Thanks, > > Meng Zhu > >
