> On Feb. 19, 2016, 10:24 a.m., Alexander Rukletsov wrote: > > src/master/master.hpp, lines 1530-1531 > > <https://reviews.apache.org/r/41681/diff/32/?file=1241338#file1241338line1530> > > > > Let's pull at least the implementation out of the header. You may look > > at maintenance and quota for inspiration : ). > > > > Going further, how about extracting registry part into a separate > > patch? It will make reviewing easier. Also, it would be great to see some > > registry tests as well. > > Yongqiao Wang wrote: > Thanks Alex. Personally, I find it more difficult to review a chain of > commits than a single, a single patch can give a whole picture of a feature > at a time. In addition, the main reason is Adam has completed to review this > patch, if I split it into some small patches, then maybe Adam needs to review > them again before commiting them. Can we keep this patch except make the > smaller chagnes for addressing comments? can you just review the changes to > each file one at a time in the this commit? Based on your comments, I will > post another patch for registery test later. > > For moving the implementation out of the header, I will update this > patches later. > > Adam B wrote: > I agree with Yongqioa. While patches with <100 lines are quicker/easier > to review, this patch is only about 300 new/changed lines, and I like to > think I've reviewed it pretty thoroughly already. Please review the patch as > is, and let us know if there's anything for which we should block the commit. > We can pull the implementation out of the header in a separate, follow-up > patch. > We can also add additional registry tests and/or authz tests in a > follow-up patch, if they aren't already covered in > https://reviews.apache.org/r/41790/ > In the future, we can encourage Yongqiao to break up larger patches > earlier in the review process.
I have posted another patch https://reviews.apache.org/r/43863/ to move the implementation of updateWeights operation out of the header. - Yongqiao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41681/#review119857 ----------------------------------------------------------- On Feb. 14, 2016, 12:02 p.m., Yongqiao Wang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41681/ > ----------------------------------------------------------- > > (Updated Feb. 14, 2016, 12:02 p.m.) > > > Review request for mesos, Adam B, Neil Conway, and Qian Zhang. > > > Bugs: MESOS-4214 > https://issues.apache.org/jira/browse/MESOS-4214 > > > Repository: mesos > > > Description > ------- > > Introduce HTTP endpoint /weights for updating weight. > > > Diffs > ----- > > include/mesos/authorizer/authorizer.hpp > 5ee3c7afadd131802c93febbb6b4dbad069c2d81 > include/mesos/authorizer/authorizer.proto > 226441f8cbd6d0828bf1636cc08c21ffcc75e6a7 > src/CMakeLists.txt 9ab84c0898b3adce6063cc50b04ee74cf1471609 > src/Makefile.am 5813ab2c33a7de6b612064e894e5f15b5a474e2b > src/authorizer/local/authorizer.hpp > c7321c276d566eca6a91f45c546468bea1b0da15 > src/authorizer/local/authorizer.cpp > 9557bbdf68ff182c4538bbf70cee576d717abc05 > src/master/http.cpp f92212bf69f9db51d729347fb553e74e28e105fd > src/master/master.hpp 2f2ad2ada508e1923bf995ab124367a3b082b572 > src/master/master.cpp e1ca81dab85a7ab1391eca0d6bd995548bf79c22 > src/master/registry.proto 9958f9c2bdb785390fca2f292b65d5a9310434d5 > src/master/weights_handler.cpp PRE-CREATION > src/tests/mesos.hpp 242a11658c0a9ba4caced9b2b2bdbcb921f7fdd0 > src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c > > Diff: https://reviews.apache.org/r/41681/diff/ > > > Testing > ------- > > Make & Make check successfully! > > $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master > --weights="role1=4.2,role2=3.1" --authenticate_http > --credentials=/opt/credentials.json >> /tmp/mesos-master.log 2>&1 &) > $ curl http://localhost:5050/roles | python -mjson.tool > { > "roles": [ > { > "frameworks": [ ], > "name": "*", > "resources": { > "cpus": 0, > "disk": 0, > "mem": 0 > }, > "weight": 1 > }, > { > "frameworks": [ ], > "name": "role1", > "resources": { > "cpus": 0, > "disk": 0, > "mem": 0 > }, > "weight": 4.2 > }, > { > "frameworks": [ ], > "name": "role2", > "resources": { > "cpus": 0, > "disk": 0, > "mem": 0 > }, > "weight": 3.1 > } > ] > } > > Test update: > $ curl --user framework1:secret_string1 --data > "[{\"weight\":1.8,\"role\":\"role1\"},{\"weight\":1.0,\"role\":\"role2\"},{\"weight\":3.4,\"role\":\"role3\"}]" > -X PUT http://127.0.0.1:5050/weights > $ curl http://localhost:5050/roles | python -mjson.tool > { > "roles": [ > { > "frameworks": [], > "name": "*", > "resources": { > "cpus": 0, > "disk": 0, > "mem": 0 > }, > "weight": 1.0 > }, > { > "frameworks": [], > "name": "role1", > "resources": { > "cpus": 0, > "disk": 0, > "mem": 0 > }, > "weight": 1.8 > }, > { > "frameworks": [], > "name": "role2", > "resources": { > "cpus": 0, > "disk": 0, > "mem": 0 > }, > "weight": 1.0 > }, > { > "frameworks": [], > "name": "role3", > "resources": { > "cpus": 0, > "disk": 0, > "mem": 0 > }, > "weight": 3.4 > } > ] > } > > Test recovuery: > $ ps -ef | grep mesos-master > 501 56292 1 0 6:18PM ttys001 0:00.31 > /Users/yqwyq/Desktop/mesos/build/src/.libs/mesos-master --ip=127.0.0.1 > --work_dir=/tmp/mesos-master --weights=role1=4.2,role2=3.1 > --authenticate_http --credentials=/opt/credentials.json > $ kill -9 56292 > > $ (./mesos-master.sh --ip=127.0.0.1 --work_dir=/tmp/mesos-master > --weights="role1=4.2,role2=3.1,role6=9.0" --authenticate_http > --credentials=/opt/credentials.json >> /tmp/mesos-master.log 2>&1 &) > $ curl http://localhost:5050/roles | python -mjson.tool > { > "roles": [ > { > "frameworks": [], > "name": "*", > "resources": { > "cpus": 0, > "disk": 0, > "mem": 0 > }, > "weight": 1.0 > }, > { > "frameworks": [], > "name": "role1", > "resources": { > "cpus": 0, > "disk": 0, > "mem": 0 > }, > "weight": 1.8 > }, > { > "frameworks": [], > "name": "role2", > "resources": { > "cpus": 0, > "disk": 0, > "mem": 0 > }, > "weight": 1.0 > }, > { > "frameworks": [], > "name": "role3", > "resources": { > "cpus": 0, > "disk": 0, > "mem": 0 > }, > "weight": 3.4 > } > ] > } > > > Thanks, > > Yongqiao Wang > >
