You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Quinn Leng <qu...@gmail.com> on 2017/06/30 22:18:10 UTC
Review Request 60580: Added filtering to the '/frameworks' endpoint.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60580/
-----------------------------------------------------------
Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
Bugs: MESOS-7630
https://issues.apache.org/jira/browse/MESOS-7630
Repository: mesos
Description
-------
Added filtering to the '/frameworks' endpoint.
Add query parameter support for '/frameworks' endpoint.
To query the information about a specific framework, we
can use 'framework_id' to specify the framework, thus it
looks like'/frameworks?framework_id=[framework id]'
To maintain consistency, the structure for response is
reserved, thus the response still contains field 'frameworks'
and 'completed_frameworks' etc..
Since code for '/state' endpoint and '/frameworks' are coupled
(they share 'FullFrameworkWriter' class), thus the code for '/state'
endpoint is also updated.
Diffs
-----
src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e
src/common/http.cpp 2f7718cbc2e449b4f7c89754e8f84eac2f3c35b6
src/master/http.cpp 4dd43fd7c3fb986f4eed78bce574b6d3af156b67
src/tests/master_tests.cpp 6cd4be7e5ba48c6562ce91b28e76b065522cfbea
Diff: https://reviews.apache.org/r/60580/diff/1/
Testing
-------
Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
Thanks,
Quinn Leng
Re: Review Request 60580: Added filtering to the '/frameworks'
endpoint.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60580/#review179731
-----------------------------------------------------------
Bad patch!
Reviews applied: [60580]
Failed command: python support/apply-reviews.py -n -r 60580
Error:
Traceback (most recent call last):
File "support/apply-reviews.py", line 417, in <module>
main()
File "support/apply-reviews.py", line 412, in main
reviewboard(options)
File "support/apply-reviews.py", line 402, in reviewboard
apply_review(options)
File "support/apply-reviews.py", line 158, in apply_review
fetch_patch(options)
File "support/apply-reviews.py", line 215, in fetch_patch
shell(cmd, options['dry_run'])
UnboundLocalError: local variable 'cmd' referenced before assignment
Full log: http://mesos-winbot.westus.cloudapp.azure.com/logs/125/console
- Mesos Reviewbot Windows
On July 5, 2017, 4:30 p.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60580/
> -----------------------------------------------------------
>
> (Updated July 5, 2017, 4:30 p.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added filtering to the '/frameworks' endpoint.
>
> Add query parameter support for '/frameworks' endpoint.
> To query the information about a specific framework, we
> can use 'framework_id' to specify the framework, thus it
> looks like'/frameworks?framework_id=[framework id]'
>
> To maintain consistency, the structure for response is
> reserved, thus the response still contains field 'frameworks'
> and 'completed_frameworks' etc..
>
> Since code for '/state' endpoint and '/frameworks' are coupled
> (they share 'FullFrameworkWriter' class), thus the code for '/state'
> endpoint is also updated.
>
>
> Diffs
> -----
>
> src/common/http.hpp b7e4a8adcbcaa3a962af795c67694a35161b6c1a
> src/common/http.cpp fdb591eccf273260902f3f695cf431f72ee3d817
> src/master/http.cpp 64b7cdd1c42a067bf24e302b9496bb75cb916590
> src/tests/master_tests.cpp d21194fb19781a24223d6b0d8f99987f4edb5a9a
>
>
> Diff: https://reviews.apache.org/r/60580/diff/2/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60580: Added filtering to the '/frameworks'
endpoint.
Posted by Quinn Leng <qu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60580/
-----------------------------------------------------------
(Updated July 13, 2017, 8:12 p.m.)
Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
Bugs: MESOS-7630
https://issues.apache.org/jira/browse/MESOS-7630
Repository: mesos
Description
-------
Added filtering to the '/frameworks' endpoint.
Add query parameter support for '/frameworks' endpoint.
To query the information about a specific framework, we
can use 'framework_id' to specify the framework, thus it
looks like'/frameworks?framework_id=[framework id]'
To maintain consistency, the structure for response is
reserved, thus the response still contains field 'frameworks'
and 'completed_frameworks' etc..
Since code for '/state' endpoint and '/frameworks' are coupled
(they share 'FullFrameworkWriter' class), thus the code for '/state'
endpoint is also updated.
Diffs
-----
src/master/http.cpp 948aa118101b6ce03410c9e0c945b6ca16668ca2
src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
Diff: https://reviews.apache.org/r/60580/diff/4/
Testing
-------
Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
Thanks,
Quinn Leng
Re: Review Request 60580: Added filtering to the '/frameworks'
endpoint.
Posted by Quinn Leng <qu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60580/
-----------------------------------------------------------
(Updated July 12, 2017, 6:19 p.m.)
Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
Bugs: MESOS-7630
https://issues.apache.org/jira/browse/MESOS-7630
Repository: mesos
Description
-------
Added filtering to the '/frameworks' endpoint.
Add query parameter support for '/frameworks' endpoint.
To query the information about a specific framework, we
can use 'framework_id' to specify the framework, thus it
looks like'/frameworks?framework_id=[framework id]'
To maintain consistency, the structure for response is
reserved, thus the response still contains field 'frameworks'
and 'completed_frameworks' etc..
Since code for '/state' endpoint and '/frameworks' are coupled
(they share 'FullFrameworkWriter' class), thus the code for '/state'
endpoint is also updated.
Diffs (updated)
-----
src/master/http.cpp 948aa118101b6ce03410c9e0c945b6ca16668ca2
src/tests/master_tests.cpp 6e6461c2e13c3eb055aa3c2d8ad8e3ac54a1d197
Diff: https://reviews.apache.org/r/60580/diff/4/
Changes: https://reviews.apache.org/r/60580/diff/3-4/
Testing
-------
Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
Thanks,
Quinn Leng
Re: Review Request 60580: Added filtering to the '/frameworks'
endpoint.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60580/#review180248
-----------------------------------------------------------
src/tests/master_tests.cpp
Lines 5669-5670 (patched)
<https://reviews.apache.org/r/60580/#comment255327>
The intention is for frameworks to subscribe initially without a framework ID, and the master then assigns them a framework ID. Unfortunately, we currently allow frameworks to register with unrecognized framework IDs because the master doesn't persist existing framework infos.
So it would be better here to follow the intended pattern and subscribe without a framework ID in the FrameworkInfo. Then, you can extract the framework ID from the `registered` call and add it to your `expected` JSON blob.
- Greg Mann
On July 7, 2017, 11:20 p.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60580/
> -----------------------------------------------------------
>
> (Updated July 7, 2017, 11:20 p.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added filtering to the '/frameworks' endpoint.
>
> Add query parameter support for '/frameworks' endpoint.
> To query the information about a specific framework, we
> can use 'framework_id' to specify the framework, thus it
> looks like'/frameworks?framework_id=[framework id]'
>
> To maintain consistency, the structure for response is
> reserved, thus the response still contains field 'frameworks'
> and 'completed_frameworks' etc..
>
> Since code for '/state' endpoint and '/frameworks' are coupled
> (they share 'FullFrameworkWriter' class), thus the code for '/state'
> endpoint is also updated.
>
>
> Diffs
> -----
>
> src/master/http.cpp 175a44ce7fb5be509453c25eaa9ec29f35adba3a
> src/tests/master_tests.cpp c778c6c56d47c4033189912cebee6024be79106f
>
>
> Diff: https://reviews.apache.org/r/60580/diff/3/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60580: Added filtering to the '/frameworks'
endpoint.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60580/#review180220
-----------------------------------------------------------
I still need to look at the test, but find below comments on the functional changes.
Also, it looks like this patch currently has conflicts - could you rebase?
src/master/http.cpp
Lines 1353 (patched)
<https://reviews.apache.org/r/60580/#comment255273>
This last empty string can be omitted.
src/master/http.cpp
Lines 1415-1417 (original), 1416 (patched)
<https://reviews.apache.org/r/60580/#comment255278>
Let's retain the pattern of explicitly capturing the acceptors by reference. This allows us to avoid copying `Owned` objects.
src/master/http.cpp
Lines 1437-1439 (original), 1437 (patched)
<https://reviews.apache.org/r/60580/#comment255279>
Ditto regarding by-reference capture.
src/master/http.cpp
Lines 2821-2822 (original), 2804 (patched)
<https://reviews.apache.org/r/60580/#comment255285>
Ditto regarding by-reference capture.
src/master/http.cpp
Lines 2842-2843 (original), 2825 (patched)
<https://reviews.apache.org/r/60580/#comment255286>
Ditto regarding by-reference capture.
src/tests/master_tests.cpp
Lines 5660 (patched)
<https://reviews.apache.org/r/60580/#comment255293>
s/query parameter/a query parameter/
src/tests/master_tests.cpp
Lines 5661 (patched)
<https://reviews.apache.org/r/60580/#comment255295>
s/correct number and content of framework/the correct framework/
- Greg Mann
On July 7, 2017, 11:20 p.m., Quinn Leng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60580/
> -----------------------------------------------------------
>
> (Updated July 7, 2017, 11:20 p.m.)
>
>
> Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
>
>
> Bugs: MESOS-7630
> https://issues.apache.org/jira/browse/MESOS-7630
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added filtering to the '/frameworks' endpoint.
>
> Add query parameter support for '/frameworks' endpoint.
> To query the information about a specific framework, we
> can use 'framework_id' to specify the framework, thus it
> looks like'/frameworks?framework_id=[framework id]'
>
> To maintain consistency, the structure for response is
> reserved, thus the response still contains field 'frameworks'
> and 'completed_frameworks' etc..
>
> Since code for '/state' endpoint and '/frameworks' are coupled
> (they share 'FullFrameworkWriter' class), thus the code for '/state'
> endpoint is also updated.
>
>
> Diffs
> -----
>
> src/master/http.cpp 175a44ce7fb5be509453c25eaa9ec29f35adba3a
> src/tests/master_tests.cpp c778c6c56d47c4033189912cebee6024be79106f
>
>
> Diff: https://reviews.apache.org/r/60580/diff/3/
>
>
> Testing
> -------
>
> Passed 'make check -j48'
> Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
> Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
>
>
> Thanks,
>
> Quinn Leng
>
>
Re: Review Request 60580: Added filtering to the '/frameworks'
endpoint.
Posted by Quinn Leng <qu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60580/
-----------------------------------------------------------
(Updated July 7, 2017, 11:20 p.m.)
Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
Bugs: MESOS-7630
https://issues.apache.org/jira/browse/MESOS-7630
Repository: mesos
Description
-------
Added filtering to the '/frameworks' endpoint.
Add query parameter support for '/frameworks' endpoint.
To query the information about a specific framework, we
can use 'framework_id' to specify the framework, thus it
looks like'/frameworks?framework_id=[framework id]'
To maintain consistency, the structure for response is
reserved, thus the response still contains field 'frameworks'
and 'completed_frameworks' etc..
Since code for '/state' endpoint and '/frameworks' are coupled
(they share 'FullFrameworkWriter' class), thus the code for '/state'
endpoint is also updated.
Diffs (updated)
-----
src/master/http.cpp 175a44ce7fb5be509453c25eaa9ec29f35adba3a
src/tests/master_tests.cpp c778c6c56d47c4033189912cebee6024be79106f
Diff: https://reviews.apache.org/r/60580/diff/3/
Changes: https://reviews.apache.org/r/60580/diff/2-3/
Testing
-------
Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
Thanks,
Quinn Leng
Re: Review Request 60580: Added filtering to the '/frameworks'
endpoint.
Posted by Quinn Leng <qu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60580/
-----------------------------------------------------------
(Updated July 5, 2017, 4:30 p.m.)
Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
Bugs: MESOS-7630
https://issues.apache.org/jira/browse/MESOS-7630
Repository: mesos
Description (updated)
-------
Added filtering to the '/frameworks' endpoint.
Add query parameter support for '/frameworks' endpoint.
To query the information about a specific framework, we
can use 'framework_id' to specify the framework, thus it
looks like'/frameworks?framework_id=[framework id]'
To maintain consistency, the structure for response is
reserved, thus the response still contains field 'frameworks'
and 'completed_frameworks' etc..
Since code for '/state' endpoint and '/frameworks' are coupled
(they share 'FullFrameworkWriter' class), thus the code for '/state'
endpoint is also updated.
Diffs
-----
src/common/http.hpp b7e4a8adcbcaa3a962af795c67694a35161b6c1a
src/common/http.cpp fdb591eccf273260902f3f695cf431f72ee3d817
src/master/http.cpp 64b7cdd1c42a067bf24e302b9496bb75cb916590
src/tests/master_tests.cpp d21194fb19781a24223d6b0d8f99987f4edb5a9a
Diff: https://reviews.apache.org/r/60580/diff/2/
Testing
-------
Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
Thanks,
Quinn Leng
Re: Review Request 60580: Added filtering to the '/frameworks'
endpoint.
Posted by Quinn Leng <qu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60580/
-----------------------------------------------------------
(Updated July 3, 2017, 8:10 p.m.)
Review request for mesos, Anand Mazumdar, Alexander Rojas, Greg Mann, and Vinod Kone.
Bugs: MESOS-7630
https://issues.apache.org/jira/browse/MESOS-7630
Repository: mesos
Description (updated)
-------
Added filtering to the '/frameworks' endpoint.
Diffs (updated)
-----
src/common/http.hpp b7e4a8adcbcaa3a962af795c67694a35161b6c1a
src/common/http.cpp fdb591eccf273260902f3f695cf431f72ee3d817
src/master/http.cpp 64b7cdd1c42a067bf24e302b9496bb75cb916590
src/tests/master_tests.cpp d21194fb19781a24223d6b0d8f99987f4edb5a9a
Diff: https://reviews.apache.org/r/60580/diff/2/
Changes: https://reviews.apache.org/r/60580/diff/1-2/
Testing
-------
Passed 'make check -j48'
Passed 'GTEST_FILTER="MasterTest.FrameworksEndpointQueryFramework" make check -j48'
Passed 'GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="MasterTest.FrameworksEndpointQueryFramework" --gtest_repeat=1000 --gtest_break_on_failure'
Thanks,
Quinn Leng