You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Kapil Arya <ka...@mesosphere.io> on 2018/05/04 17:33:06 UTC
Re: Review Request 66230: Added test for adding/removing framework
roles.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66230/
-----------------------------------------------------------
(Updated May 4, 2018, 1:33 p.m.)
Review request for mesos, Benjamin Mahler and Till Toenshoff.
Changes
-------
Enhanced tests.
Bugs: MESOS-7258
https://issues.apache.org/jira/browse/MESOS-7258
Repository: mesos
Description
-------
Added test for adding/removing framework roles.
Diffs (updated)
-----
src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af
Diff: https://reviews.apache.org/r/66230/diff/8/
Changes: https://reviews.apache.org/r/66230/diff/7-8/
Testing
-------
Thanks,
Kapil Arya
Re: Review Request 66230: Added test for adding/removing framework
roles.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66230/#review202461
-----------------------------------------------------------
FAIL: Failed to apply the dependent review: 66229.
Failed command: `python.exe .\support\apply-reviews.py -n -r 66229`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66230
Relevant logs:
- [apply-review-66229-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66230/logs/apply-review-66229-stdout.log):
```
error: patch failed: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp:684
error: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp: patch does not apply
```
- Mesos Reviewbot Windows
On May 4, 2018, 5:33 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66230/
> -----------------------------------------------------------
>
> (Updated May 4, 2018, 5:33 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
>
>
> Bugs: MESOS-7258
> https://issues.apache.org/jira/browse/MESOS-7258
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added test for adding/removing framework roles.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af
>
>
> Diff: https://reviews.apache.org/r/66230/diff/8/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 66230: Added test for updating framework info.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66230/#review202813
-----------------------------------------------------------
FAIL: Failed to apply the dependent review: 66229.
Failed command: `python.exe .\support\apply-reviews.py -n -r 66229`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66230
Relevant logs:
- [apply-review-66229-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66230/logs/apply-review-66229-stdout.log):
```
error: patch failed: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp:684
error: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp: patch does not apply
```
- Mesos Reviewbot Windows
On May 9, 2018, 6:15 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66230/
> -----------------------------------------------------------
>
> (Updated May 9, 2018, 6:15 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
>
>
> Bugs: MESOS-7258
> https://issues.apache.org/jira/browse/MESOS-7258
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Includes tests to validate UPDATE_FRAMEWORK call as well as specific
> tests for adding/removing framework roles.
>
>
> Diffs
> -----
>
> src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad
> src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af
>
>
> Diff: https://reviews.apache.org/r/66230/diff/9/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 66230: Added test for updating framework info.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66230/#review202833
-----------------------------------------------------------
Bad patch!
Reviews applied: [66230, 66229, 66228, 67049, 67048, 67047, 66872]
Failed command: python support/apply-reviews.py -n -r 66229
Error:
2018-05-10 11:48:03 URL:https://reviews.apache.org/r/66229/diff/raw/ [4868/4868] -> "66229.patch" [1]
error: patch failed: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp:684
error: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp: patch does not apply
Full log: https://builds.apache.org/job/Mesos-Reviewbot/22420/console
- Mesos Reviewbot
On May 9, 2018, 6:15 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66230/
> -----------------------------------------------------------
>
> (Updated May 9, 2018, 6:15 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
>
>
> Bugs: MESOS-7258
> https://issues.apache.org/jira/browse/MESOS-7258
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Includes tests to validate UPDATE_FRAMEWORK call as well as specific
> tests for adding/removing framework roles.
>
>
> Diffs
> -----
>
> src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad
> src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af
>
>
> Diff: https://reviews.apache.org/r/66230/diff/9/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 66230: Added test for updating framework info.
Posted by Kapil Arya <ka...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66230/
-----------------------------------------------------------
(Updated May 9, 2018, 9:15 p.m.)
Review request for mesos, Benjamin Mahler and Till Toenshoff.
Summary (updated)
-----------------
Added test for updating framework info.
Bugs: MESOS-7258
https://issues.apache.org/jira/browse/MESOS-7258
Repository: mesos
Description (updated)
-------
Includes tests to validate UPDATE_FRAMEWORK call as well as specific
tests for adding/removing framework roles.
Diffs (updated)
-----
src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad
src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af
Diff: https://reviews.apache.org/r/66230/diff/9/
Changes: https://reviews.apache.org/r/66230/diff/8-9/
Testing
-------
Thanks,
Kapil Arya
Re: Review Request 66230: Added test for adding/removing framework
roles.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66230/#review202499
-----------------------------------------------------------
Bad patch!
Reviews applied: [66230, 66229, 66228, 66872]
Failed command: python support/apply-reviews.py -n -r 66229
Error:
2018-05-04 22:50:30 URL:https://reviews.apache.org/r/66229/diff/raw/ [12365/12365] -> "66229.patch" [1]
error: patch failed: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp:684
error: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp: patch does not apply
Full log: https://builds.apache.org/job/Mesos-Reviewbot/22373/console
- Mesos Reviewbot
On May 4, 2018, 5:33 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66230/
> -----------------------------------------------------------
>
> (Updated May 4, 2018, 5:33 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
>
>
> Bugs: MESOS-7258
> https://issues.apache.org/jira/browse/MESOS-7258
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added test for adding/removing framework roles.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af
>
>
> Diff: https://reviews.apache.org/r/66230/diff/8/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kapil Arya
>
>
Re: Review Request 66230: Added test for adding/removing framework
roles.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66230/#review202533
-----------------------------------------------------------
Can we simplify these tests and remove the executors?
Also, over the course of reviewing this I realized that role removal should rescind offers for the removed roles and I don't think these patches address that?
src/tests/scheduler_tests.cpp
Lines 1938 (patched)
<https://reviews.apache.org/r/66230/#comment284346>
Are you setting this in order to test a non-whitelisted role? If so, maybe a short comment here?
```
// Set the whitelist in order to test a non-whitelisted role
// is considered invalid.
```
src/tests/scheduler_tests.cpp
Lines 1946-1949 (patched)
<https://reviews.apache.org/r/66230/#comment284350>
Why do you need an executor and test containerizer for this test? Can we avoid it?
src/tests/scheduler_tests.cpp
Lines 1951-1956 (patched)
<https://reviews.apache.org/r/66230/#comment284349>
Are you using this to test that the update makes its way to the allocator? Maybe a short note?
```
// We add an agent in order to test the framework update
// makes its way to the allocator.
```
src/tests/scheduler_tests.cpp
Lines 2201 (patched)
<https://reviews.apache.org/r/66230/#comment284345>
End with a period? Also, sometimes the comments quote the roles and sometimes they don't quote them (like here), can you make it consistent?
src/tests/scheduler_tests.cpp
Lines 2201-2221 (patched)
<https://reviews.apache.org/r/66230/#comment284351>
This should probably rescind offers to the removed role, can we test that? I'm guessing that's a bug in the call implementation, can you address it in a subsequent patch?
- Benjamin Mahler
On May 4, 2018, 5:33 p.m., Kapil Arya wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66230/
> -----------------------------------------------------------
>
> (Updated May 4, 2018, 5:33 p.m.)
>
>
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
>
>
> Bugs: MESOS-7258
> https://issues.apache.org/jira/browse/MESOS-7258
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added test for adding/removing framework roles.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af
>
>
> Diff: https://reviews.apache.org/r/66230/diff/8/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Kapil Arya
>
>