> On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 73 (patched) > > <https://reviews.apache.org/r/56213/diff/3/?file=1627707#file1627707line73> > > > > Any particular reason for using v1 scheduler API? The general > > recommendation has been to use v0 scheduler API unless you are testing v1 > > features. The main reason being most users are still using v0 scheduler API. > > Alexander Rukletsov wrote: > My original intention was to use unversioned HTTP API. However, I didn't > figure out how to do this and I suspect that it is not even possible. Is it > correct, that there are two choices? > 1) use older driver-based API > 2) use newer v1 HTTP API > > Is the recommendation then to use older driver-based API in tests?
yes, the recommendation is to use v0 scheduler API unless either the rest of the tests in the file are all using v1 scheduler API or you are testing v1 scheduler API functionality. > On Feb. 10, 2017, 2:42 a.m., Vinod Kone wrote: > > src/tests/check_tests.cpp > > Lines 177 (patched) > > <https://reviews.apache.org/r/56213/diff/3/?file=1627707#file1627707line177> > > > > s/agent/slave/ > > > > here and everywhere else. > > > > we are not doing this change yet. > > Gastón Kleiman wrote: > We do this in `health_check_tests.cpp`: > > ``` > $ grep "agent = " health_check_tests.cpp | wc -l > 15 > $ grep "slave = " health_check_tests.cpp | wc -l > 0 > $ > ``` > > Vinod Kone wrote: > I think we should fix `health_check_tests.cpp` as well then. > > Gastón Kleiman wrote: > I used `health_check_tests.cpp` just as an example, but we're using this > in a lot of places: > > ``` > $ git grep "agent[^ ]* =" | cut -d: -f1 | sort | uniq > api_tests.cpp > authentication_tests.cpp > containerizer/cgroups_isolator_tests.cpp > containerizer/docker_containerizer_tests.cpp > containerizer/io_switchboard_tests.cpp > containerizer/memory_pressure_tests.cpp > fault_tolerance_tests.cpp > health_check_tests.cpp > hierarchical_allocator_tests.cpp > hook_tests.cpp > master_allocator_tests.cpp > master_tests.cpp > master_validation_tests.cpp > mesos.cpp > mesos.hpp > metrics_tests.cpp > oversubscription_tests.cpp > partition_tests.cpp > reconciliation_tests.cpp > reservation_endpoints_tests.cpp > reservation_tests.cpp > slave_authorization_tests.cpp > slave_recovery_tests.cpp > slave_tests.cpp > sorter_tests.cpp > $ git grep "agent[^ ]* =" | wc -l > 207 > $ > ``` > > Alexander Rukletsov wrote: > The policy I'm sticking to is to use `agent` in all new or being modified > non-user facing code. I'd suggest we keep `agent` here and update in other > places as we go. I spot checked some of these files, and looks like the code has been updated recently to rename `slaveFlags` to `agentFlags` (which I suspect is what the grep above is capturing) which is unfortunate. I think this arose from a single commit that Alex Clemmer did. It's my fault to not clearly articulate on dev list how phase 2 of slave to agent rename should be done. But the policy that I've been suggesting to my reviwees, is to not make a file locally inconsistent. IOW, if the rest of the file is calling it `agent` use `agent`, otherwise stick to `slave`. I think this applies to any such change not just slave to agent rename. The reason being, a new dev coming to a file and adding a new test, would be very confused if a file has a mix of old and new style. So, if you really want to use `agent` here, I recommend to do a sweep to update this file to do it across all tests (in a dependent review of course). Does that make sense? - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56213/#review165049 ----------------------------------------------------------- On March 1, 2017, 12:50 p.m., Alexander Rukletsov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56213/ > ----------------------------------------------------------- > > (Updated March 1, 2017, 12:50 p.m.) > > > Review request for mesos, Gastón Kleiman and Vinod Kone. > > > Bugs: MESOS-6906 > https://issues.apache.org/jira/browse/MESOS-6906 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > src/tests/check_tests.cpp f035c16920deaf559420ae0d7d881330ff65ae44 > src/tests/mesos.hpp b450a04dfbf3bbeeb6b605fb78097dca390cbdbe > > > Diff: https://reviews.apache.org/r/56213/diff/4/ > > > Testing > ------- > > See https://reviews.apache.org/r/56218/ > > > Thanks, > > Alexander Rukletsov > >
