You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Greg Mann <gr...@mesosphere.io> on 2018/03/02 08:07:54 UTC
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/
-----------------------------------------------------------
(Updated March 2, 2018, 8:07 a.m.)
Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
Changes
-------
Rebase.
Bugs: MESOS-8190
https://issues.apache.org/jira/browse/MESOS-8190
Repository: mesos
Description (updated)
-------
This patch adds a new test,
'SchedulerTest.OfferOperationFeedbackValidation',
to verify that task and offer operation status updates are correctly
sent when the 'id' field is set incorrectly on an operation.
Diffs (updated)
-----
src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
Diff: https://reviews.apache.org/r/63994/diff/2/
Changes: https://reviews.apache.org/r/63994/diff/1-2/
Testing
-------
make check
Thanks,
Greg Mann
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/#review198571
-----------------------------------------------------------
PASS: Mesos patch 63994 was successfully built and tested.
Reviews applied: `['63991', '63992', '63994']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63994
- Mesos Reviewbot Windows
On March 2, 2018, 2:52 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> -----------------------------------------------------------
>
> (Updated March 2, 2018, 2:52 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
>
>
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new test,
> 'SchedulerTest.OfferOperationFeedbackValidation',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
>
>
> Diff: https://reviews.apache.org/r/63994/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/#review199125
-----------------------------------------------------------
PASS: Mesos patch 63994 was successfully built and tested.
Reviews applied: `['63991', '63992', '63994']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63994
- Mesos Reviewbot Windows
On March 13, 2018, 9:48 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> -----------------------------------------------------------
>
> (Updated March 13, 2018, 9:48 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
>
>
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds two new scheduler tests,
> 'OperationFeedbackValidationWithResourceProviderCapability' and
> 'OperationFeedbackValidationNoResourceProviderCapability',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
>
>
> Diff: https://reviews.apache.org/r/63994/diff/5/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/
-----------------------------------------------------------
(Updated March 16, 2018, 1:55 a.m.)
Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
Bugs: MESOS-8190
https://issues.apache.org/jira/browse/MESOS-8190
Repository: mesos
Description (updated)
-------
This patch adds two new scheduler tests,
'OperationFeedbackValidationWithResourceProviderCapability' and
'OperationFeedbackValidationNoResourceProviderCapability',
to verify that task and offer operation status updates are correctly
sent when the 'id' field is set incorrectly on an operation.
Diffs (updated)
-----
src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
Diff: https://reviews.apache.org/r/63994/diff/7/
Changes: https://reviews.apache.org/r/63994/diff/6-7/
Testing
-------
make check
Thanks,
Greg Mann
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Greg Mann <gr...@mesosphere.io>.
> On March 16, 2018, 12:23 a.m., Gaston Kleiman wrote:
> > src/tests/scheduler_tests.cpp
> > Lines 1239-1240 (patched)
> > <https://reviews.apache.org/r/63994/diff/5-6/?file=1974906#file1974906line1245>
> >
> > Do we need this? My understanding is that frameworks will get offers as soon as they register, without having to manually advance the clock.
Yep I tested this - without the advance, the offer will not come.
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/#review199305
-----------------------------------------------------------
On March 16, 2018, 12:06 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> -----------------------------------------------------------
>
> (Updated March 16, 2018, 12:06 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
>
>
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds three new scheduler tests,
> 'OperationFeedbackValidationWithResourceProviderCapability',
> 'OperationFeedbackValidationNoResourceProviderCapability', and
> 'OperationFeedbackValidationSchedulerDriverFramework', to verify
> that task and operation status updates, and error messages, are
> correctly sent when the 'id' field is set incorrectly on an
> operation.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
>
>
> Diff: https://reviews.apache.org/r/63994/diff/6/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/#review199305
-----------------------------------------------------------
src/tests/scheduler_tests.cpp
Lines 1239-1240 (patched)
<https://reviews.apache.org/r/63994/#comment279602>
Do we need this? My understanding is that frameworks will get offers as soon as they register, without having to manually advance the clock.
src/tests/scheduler_tests.cpp
Lines 1251-1256 (patched)
<https://reviews.apache.org/r/63994/#comment279601>
The following is shorter/more readable:
```
Resources resources = Resources::parse("cpus:0.1")->pushReservation(createDynamicReservationInfo(
"RESERVED_ROLE"));
```
- Gaston Kleiman
On March 15, 2018, 5:06 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> -----------------------------------------------------------
>
> (Updated March 15, 2018, 5:06 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
>
>
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds three new scheduler tests,
> 'OperationFeedbackValidationWithResourceProviderCapability',
> 'OperationFeedbackValidationNoResourceProviderCapability', and
> 'OperationFeedbackValidationSchedulerDriverFramework', to verify
> that task and operation status updates, and error messages, are
> correctly sent when the 'id' field is set incorrectly on an
> operation.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
>
>
> Diff: https://reviews.apache.org/r/63994/diff/6/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/
-----------------------------------------------------------
(Updated March 16, 2018, 12:06 a.m.)
Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
Bugs: MESOS-8190
https://issues.apache.org/jira/browse/MESOS-8190
Repository: mesos
Description (updated)
-------
This patch adds three new scheduler tests,
'OperationFeedbackValidationWithResourceProviderCapability',
'OperationFeedbackValidationNoResourceProviderCapability', and
'OperationFeedbackValidationSchedulerDriverFramework', to verify
that task and operation status updates, and error messages, are
correctly sent when the 'id' field is set incorrectly on an
operation.
Diffs (updated)
-----
src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
Diff: https://reviews.apache.org/r/63994/diff/6/
Changes: https://reviews.apache.org/r/63994/diff/5-6/
Testing
-------
make check
Thanks,
Greg Mann
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/#review199123
-----------------------------------------------------------
Ship it!
Ship It!
- Gaston Kleiman
On March 13, 2018, 2:48 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> -----------------------------------------------------------
>
> (Updated March 13, 2018, 2:48 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
>
>
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds two new scheduler tests,
> 'OperationFeedbackValidationWithResourceProviderCapability' and
> 'OperationFeedbackValidationNoResourceProviderCapability',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
>
>
> Diff: https://reviews.apache.org/r/63994/diff/5/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/
-----------------------------------------------------------
(Updated March 13, 2018, 9:48 p.m.)
Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
Bugs: MESOS-8190
https://issues.apache.org/jira/browse/MESOS-8190
Repository: mesos
Description
-------
This patch adds two new scheduler tests,
'OperationFeedbackValidationWithResourceProviderCapability' and
'OperationFeedbackValidationNoResourceProviderCapability',
to verify that task and offer operation status updates are correctly
sent when the 'id' field is set incorrectly on an operation.
Diffs (updated)
-----
src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
Diff: https://reviews.apache.org/r/63994/diff/5/
Changes: https://reviews.apache.org/r/63994/diff/4-5/
Testing
-------
make check
Thanks,
Greg Mann
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/#review199119
-----------------------------------------------------------
src/tests/scheduler_tests.cpp
Lines 1080 (patched)
<https://reviews.apache.org/r/63994/#comment279429>
I don't think we need a block for this?
I'd remove this `{` and the `}` on line 1119.
src/tests/scheduler_tests.cpp
Lines 1081-1086 (patched)
<https://reviews.apache.org/r/63994/#comment279428>
This `call` is unused and this code should be removed.
src/tests/scheduler_tests.cpp
Lines 1185 (patched)
<https://reviews.apache.org/r/63994/#comment279430>
I'd remove these braces, unless there's a reason for this block?
- Gaston Kleiman
On March 13, 2018, 9:41 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> -----------------------------------------------------------
>
> (Updated March 13, 2018, 9:41 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
>
>
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds two new scheduler tests,
> 'OperationFeedbackValidationWithResourceProviderCapability' and
> 'OperationFeedbackValidationNoResourceProviderCapability',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
>
>
> Diff: https://reviews.apache.org/r/63994/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/#review199097
-----------------------------------------------------------
PASS: Mesos patch 63994 was successfully built and tested.
Reviews applied: `['63991', '63992', '63994']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63994
- Mesos Reviewbot Windows
On March 13, 2018, 4:41 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> -----------------------------------------------------------
>
> (Updated March 13, 2018, 4:41 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
>
>
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds two new scheduler tests,
> 'OperationFeedbackValidationWithResourceProviderCapability' and
> 'OperationFeedbackValidationNoResourceProviderCapability',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
>
>
> Diff: https://reviews.apache.org/r/63994/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/#review199116
-----------------------------------------------------------
Patch looks great!
Reviews applied: [63991, 63992, 63994]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On March 13, 2018, 9:41 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> -----------------------------------------------------------
>
> (Updated March 13, 2018, 9:41 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
>
>
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds two new scheduler tests,
> 'OperationFeedbackValidationWithResourceProviderCapability' and
> 'OperationFeedbackValidationNoResourceProviderCapability',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
>
>
> Diff: https://reviews.apache.org/r/63994/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/
-----------------------------------------------------------
(Updated March 13, 2018, 4:41 p.m.)
Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
Bugs: MESOS-8190
https://issues.apache.org/jira/browse/MESOS-8190
Repository: mesos
Description (updated)
-------
This patch adds two new scheduler tests,
'OperationFeedbackValidationWithResourceProviderCapability' and
'OperationFeedbackValidationNoResourceProviderCapability',
to verify that task and offer operation status updates are correctly
sent when the 'id' field is set incorrectly on an operation.
Diffs
-----
src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
Diff: https://reviews.apache.org/r/63994/diff/4/
Testing
-------
make check
Thanks,
Greg Mann
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/
-----------------------------------------------------------
(Updated March 13, 2018, 4:39 p.m.)
Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
Bugs: MESOS-8190
https://issues.apache.org/jira/browse/MESOS-8190
Repository: mesos
Description
-------
This patch adds a new test,
'SchedulerTest.OfferOperationFeedbackValidation',
to verify that task and offer operation status updates are correctly
sent when the 'id' field is set incorrectly on an operation.
Diffs (updated)
-----
src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
Diff: https://reviews.apache.org/r/63994/diff/4/
Changes: https://reviews.apache.org/r/63994/diff/3-4/
Testing
-------
make check
Thanks,
Greg Mann
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/#review198581
-----------------------------------------------------------
Patch looks great!
Reviews applied: [63991, 63992, 63994]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On March 2, 2018, 2:52 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> -----------------------------------------------------------
>
> (Updated March 2, 2018, 2:52 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
>
>
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new test,
> 'SchedulerTest.OfferOperationFeedbackValidation',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
>
>
> Diff: https://reviews.apache.org/r/63994/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/
-----------------------------------------------------------
(Updated March 2, 2018, 10:52 p.m.)
Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
Changes
-------
Rebase.
Bugs: MESOS-8190
https://issues.apache.org/jira/browse/MESOS-8190
Repository: mesos
Description
-------
This patch adds a new test,
'SchedulerTest.OfferOperationFeedbackValidation',
to verify that task and offer operation status updates are correctly
sent when the 'id' field is set incorrectly on an operation.
Diffs (updated)
-----
src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
Diff: https://reviews.apache.org/r/63994/diff/3/
Changes: https://reviews.apache.org/r/63994/diff/2-3/
Testing
-------
make check
Thanks,
Greg Mann
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/#review198514
-----------------------------------------------------------
FAIL: Failed to apply the dependent review: 63989.
Failed command: `python.exe .\support\apply-reviews.py -n -r 63989`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63994
Relevant logs:
- [apply-review-63989-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63994/logs/apply-review-63989-stdout.log):
```
error: patch failed: include/mesos/mesos.proto:2222
error: include/mesos/mesos.proto: patch does not apply
error: patch failed: include/mesos/v1/mesos.proto:2203
error: include/mesos/v1/mesos.proto: patch does not apply
error: patch failed: src/common/protobuf_utils.cpp:400
error: src/common/protobuf_utils.cpp: patch does not apply
error: patch failed: src/master/master.cpp:9807
error: src/master/master.cpp: patch does not apply
error: patch failed: src/slave/slave.cpp:6974
error: src/slave/slave.cpp: patch does not apply
```
- Mesos Reviewbot Windows
On March 2, 2018, 8:07 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> -----------------------------------------------------------
>
> (Updated March 2, 2018, 8:07 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
>
>
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new test,
> 'SchedulerTest.OfferOperationFeedbackValidation',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
>
>
> Diff: https://reviews.apache.org/r/63994/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/#review198526
-----------------------------------------------------------
Bad patch!
Reviews applied: [63994, 63992, 63991, 63989, 63988]
Failed command: python support/apply-reviews.py -n -r 63989
Error:
2018-03-02 13:17:17 URL:https://reviews.apache.org/r/63989/diff/raw/ [2654/2654] -> "63989.patch" [1]
error: patch failed: include/mesos/mesos.proto:2222
error: include/mesos/mesos.proto: patch does not apply
error: patch failed: include/mesos/v1/mesos.proto:2203
error: include/mesos/v1/mesos.proto: patch does not apply
error: patch failed: src/common/protobuf_utils.cpp:400
error: src/common/protobuf_utils.cpp: patch does not apply
error: patch failed: src/master/master.cpp:9807
error: src/master/master.cpp: patch does not apply
error: patch failed: src/slave/slave.cpp:6974
error: src/slave/slave.cpp: patch does not apply
Full log: https://builds.apache.org/job/Mesos-Reviewbot/21817/console
- Mesos Reviewbot
On March 2, 2018, 9:07 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> -----------------------------------------------------------
>
> (Updated March 2, 2018, 9:07 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
>
>
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new test,
> 'SchedulerTest.OfferOperationFeedbackValidation',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
>
>
> Diff: https://reviews.apache.org/r/63994/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Greg Mann
>
>
Re: Review Request 63994: Added a new test for validation of offer
operation IDs.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63994/#review198559
-----------------------------------------------------------
src/tests/scheduler_tests.cpp
Lines 1045-1047 (patched)
<https://reviews.apache.org/r/63994/#comment278756>
These and lines 1056, 1069-1078 can be replaced with:
```
EXPECT_CALL(*scheduler, connected(_))
.WillOnce(v1::scheduler::SendSubscribe(v1::DEFAULT_FRAMEWORK_INFO))
```
src/tests/scheduler_tests.cpp
Lines 1096-1142 (patched)
<https://reviews.apache.org/r/63994/#comment278757>
Can we make this shorter and more readble using the following helpers?
`v1::createTask`, `v1::createTaskGroupInfo`, `v1::LAUNCH`, `v1::LAUNCH_GROUP`, `v1::createCallAccept`, see `default_executor_tests.cpp` for multiple usage examples.
src/tests/scheduler_tests.cpp
Lines 1154-1155 (patched)
<https://reviews.apache.org/r/63994/#comment278758>
We can use this again:
```
EXPECT_CALL(*scheduler, connected(_))
.WillOnce(v1::scheduler::SendSubscribe(v1::DEFAULT_FRAMEWORK_INFO))
```
src/tests/scheduler_tests.cpp
Lines 1197-1227 (patched)
<https://reviews.apache.org/r/63994/#comment278761>
Do we really need this complicated logic?
Wouldn't it be easier to terminate the original agent and create a new one from scratch without the `RESOURCE_PROVIDER` capability?
src/tests/scheduler_tests.cpp
Lines 1247-1264 (patched)
<https://reviews.apache.org/r/63994/#comment278760>
I think that using the helpers here would also improve readability.
- Gaston Kleiman
On March 2, 2018, 12:07 a.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63994/
> -----------------------------------------------------------
>
> (Updated March 2, 2018, 12:07 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
>
>
> Bugs: MESOS-8190
> https://issues.apache.org/jira/browse/MESOS-8190
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch adds a new test,
> 'SchedulerTest.OfferOperationFeedbackValidation',
> to verify that task and offer operation status updates are correctly
> sent when the 'id' field is set incorrectly on an operation.
>
>
> Diffs
> -----
>
> src/tests/scheduler_tests.cpp 397a0908432ad50fa94fd939a604b096a06f8a1d
>
>
> Diff: https://reviews.apache.org/r/63994/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Greg Mann
>
>