You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2017/10/25 21:16:24 UTC
Review Request 63312: Updated Resources::apply for new operations.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63312/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan Schlicht.
Repository: mesos
Description
-------
This patch updated Resources::apply method to support new operations:
CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK and DESTROY_BLOCK. We
introduced an optional `convertedResources` field to this method for
new operations.
This patch also made sure that we don't call Resources::apply for
LAUNCH and LAUNCH_GROUP, because it does not really "convert" the
resources.
Diffs
-----
include/mesos/resources.hpp 9f959494f5c0376aa4ebd8f2571f4b1f45c83cb3
include/mesos/v1/resources.hpp a6216858136c82d5ebb5da3bb22c0b8a58a3b75d
src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8
src/examples/persistent_volume_framework.cpp 1f8f4bea7ce4d1a919034e0639b4f70e7c9961cf
src/master/allocator/mesos/hierarchical.cpp 5b6efe5faa3c3b10f1f714f582a155b368f8ccaf
src/v1/resources.cpp 5c0a196cf87b377b345e53a36ed24727d52381ca
Diff: https://reviews.apache.org/r/63312/diff/1/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63312: Updated Resources::apply for new operations.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63312/#review189350
-----------------------------------------------------------
FAIL: Failed to apply the current review.
Failed command: `python.exe .\support\apply-reviews.py -n -r 63312`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63312
Relevant logs:
- [apply-review-63312-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63312/logs/apply-review-63312-stdout.log):
```
error: patch failed: src/v1/resources.cpp:1814
error: src/v1/resources.cpp: patch does not apply
```
- Mesos Reviewbot Windows
On Oct. 25, 2017, 2:16 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63312/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2017, 2:16 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan Schlicht.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch updated Resources::apply method to support new operations:
> CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK and DESTROY_BLOCK. We
> introduced an optional `convertedResources` field to this method for
> new operations.
>
> This patch also made sure that we don't call Resources::apply for
> LAUNCH and LAUNCH_GROUP, because it does not really "convert" the
> resources.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp 9f959494f5c0376aa4ebd8f2571f4b1f45c83cb3
> include/mesos/v1/resources.hpp a6216858136c82d5ebb5da3bb22c0b8a58a3b75d
> src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8
> src/examples/persistent_volume_framework.cpp 1f8f4bea7ce4d1a919034e0639b4f70e7c9961cf
> src/master/allocator/mesos/hierarchical.cpp 5b6efe5faa3c3b10f1f714f582a155b368f8ccaf
> src/v1/resources.cpp 5c0a196cf87b377b345e53a36ed24727d52381ca
>
>
> Diff: https://reviews.apache.org/r/63312/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63312: Updated Resources::apply for new operations.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63312/#review189267
-----------------------------------------------------------
Bad patch!
Reviews applied: [63312, 63105, 61946, 61810, 63104, 63094, 62903, 63001]
Failed command: python support/apply-reviews.py -n -r 63312
Error:
2017-10-26 02:34:21 URL:https://reviews.apache.org/r/63312/diff/raw/ [15399/15399] -> "63312.patch" [1]
error: patch failed: src/v1/resources.cpp:1814
error: src/v1/resources.cpp: patch does not apply
Full log: https://builds.apache.org/job/Mesos-Reviewbot/19903/console
- Mesos Reviewbot
On Oct. 25, 2017, 2:16 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63312/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2017, 2:16 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan Schlicht.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch updated Resources::apply method to support new operations:
> CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK and DESTROY_BLOCK. We
> introduced an optional `convertedResources` field to this method for
> new operations.
>
> This patch also made sure that we don't call Resources::apply for
> LAUNCH and LAUNCH_GROUP, because it does not really "convert" the
> resources.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp 9f959494f5c0376aa4ebd8f2571f4b1f45c83cb3
> include/mesos/v1/resources.hpp a6216858136c82d5ebb5da3bb22c0b8a58a3b75d
> src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8
> src/examples/persistent_volume_framework.cpp 1f8f4bea7ce4d1a919034e0639b4f70e7c9961cf
> src/master/allocator/mesos/hierarchical.cpp 5b6efe5faa3c3b10f1f714f582a155b368f8ccaf
> src/v1/resources.cpp 5c0a196cf87b377b345e53a36ed24727d52381ca
>
>
> Diff: https://reviews.apache.org/r/63312/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63312: Updated Resources::apply for new operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63312/
-----------------------------------------------------------
(Updated Oct. 30, 2017, 3:25 p.m.)
Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan Schlicht.
Bugs: MESOS-7594
https://issues.apache.org/jira/browse/MESOS-7594
Repository: mesos
Description
-------
This patch updated Resources::apply method to support new operations:
CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK and DESTROY_BLOCK. We
introduced an optional `convertedResources` field to this method for
new operations.
This patch also made sure that we don't call Resources::apply for
LAUNCH and LAUNCH_GROUP, because it does not really "convert" the
resources.
Diffs
-----
include/mesos/resources.hpp 9f959494f5c0376aa4ebd8f2571f4b1f45c83cb3
include/mesos/v1/resources.hpp a6216858136c82d5ebb5da3bb22c0b8a58a3b75d
src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8
src/examples/persistent_volume_framework.cpp 1f8f4bea7ce4d1a919034e0639b4f70e7c9961cf
src/master/allocator/mesos/hierarchical.cpp 5b6efe5faa3c3b10f1f714f582a155b368f8ccaf
src/v1/resources.cpp 5c0a196cf87b377b345e53a36ed24727d52381ca
Diff: https://reviews.apache.org/r/63312/diff/3/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63312: Updated Resources::apply for new operations.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63312/#review189568
-----------------------------------------------------------
Fix it, then Ship it!
src/common/resources.cpp
Lines 1640 (patched)
<https://reviews.apache.org/r/63312/#comment266706>
Could we return an `Error` here instead? Right now we only assert for internal invariants, but not for caller-provided information.
Here, below, and in the `v1` implementation.
src/common/resources.cpp
Lines 1798 (patched)
<https://reviews.apache.org/r/63312/#comment266707>
This should be a `const` ref.
src/common/resources.cpp
Lines 1814 (patched)
<https://reviews.apache.org/r/63312/#comment266708>
This should be a `const` ref.
src/common/resources.cpp
Lines 1830 (patched)
<https://reviews.apache.org/r/63312/#comment266709>
This should be a `const` ref.
src/common/resources.cpp
Lines 1846 (patched)
<https://reviews.apache.org/r/63312/#comment266710>
This should be a `const` ref.
src/v1/resources.cpp
Lines 1827 (patched)
<https://reviews.apache.org/r/63312/#comment266711>
This should be a `const` ref.
src/v1/resources.cpp
Lines 1843 (patched)
<https://reviews.apache.org/r/63312/#comment266712>
This should be a `const` ref.
src/v1/resources.cpp
Lines 1859 (patched)
<https://reviews.apache.org/r/63312/#comment266713>
This should be a `const` ref.
src/v1/resources.cpp
Lines 1875 (patched)
<https://reviews.apache.org/r/63312/#comment266714>
This should be a `const` ref.
- Benjamin Bannier
On Oct. 29, 2017, 4:48 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63312/
> -----------------------------------------------------------
>
> (Updated Oct. 29, 2017, 4:48 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan Schlicht.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch updated Resources::apply method to support new operations:
> CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK and DESTROY_BLOCK. We
> introduced an optional `convertedResources` field to this method for
> new operations.
>
> This patch also made sure that we don't call Resources::apply for
> LAUNCH and LAUNCH_GROUP, because it does not really "convert" the
> resources.
>
>
> Diffs
> -----
>
> include/mesos/resources.hpp 9f959494f5c0376aa4ebd8f2571f4b1f45c83cb3
> include/mesos/v1/resources.hpp a6216858136c82d5ebb5da3bb22c0b8a58a3b75d
> src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8
> src/examples/persistent_volume_framework.cpp 1f8f4bea7ce4d1a919034e0639b4f70e7c9961cf
> src/master/allocator/mesos/hierarchical.cpp 5b6efe5faa3c3b10f1f714f582a155b368f8ccaf
> src/v1/resources.cpp 5c0a196cf87b377b345e53a36ed24727d52381ca
>
>
> Diff: https://reviews.apache.org/r/63312/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63312: Updated Resources::apply for new operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63312/
-----------------------------------------------------------
(Updated Oct. 29, 2017, 3:48 p.m.)
Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan Schlicht.
Changes
-------
Rebased.
Repository: mesos
Description
-------
This patch updated Resources::apply method to support new operations:
CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK and DESTROY_BLOCK. We
introduced an optional `convertedResources` field to this method for
new operations.
This patch also made sure that we don't call Resources::apply for
LAUNCH and LAUNCH_GROUP, because it does not really "convert" the
resources.
Diffs (updated)
-----
include/mesos/resources.hpp 9f959494f5c0376aa4ebd8f2571f4b1f45c83cb3
include/mesos/v1/resources.hpp a6216858136c82d5ebb5da3bb22c0b8a58a3b75d
src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8
src/examples/persistent_volume_framework.cpp 1f8f4bea7ce4d1a919034e0639b4f70e7c9961cf
src/master/allocator/mesos/hierarchical.cpp 5b6efe5faa3c3b10f1f714f582a155b368f8ccaf
src/v1/resources.cpp 5c0a196cf87b377b345e53a36ed24727d52381ca
Diff: https://reviews.apache.org/r/63312/diff/3/
Changes: https://reviews.apache.org/r/63312/diff/2-3/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63312: Updated Resources::apply for new operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63312/
-----------------------------------------------------------
(Updated Oct. 27, 2017, 10:27 a.m.)
Review request for mesos, Benjamin Bannier, Gaston Kleiman, Greg Mann, and Jan Schlicht.
Repository: mesos
Description
-------
This patch updated Resources::apply method to support new operations:
CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK and DESTROY_BLOCK. We
introduced an optional `convertedResources` field to this method for
new operations.
This patch also made sure that we don't call Resources::apply for
LAUNCH and LAUNCH_GROUP, because it does not really "convert" the
resources.
Diffs (updated)
-----
include/mesos/resources.hpp 9f959494f5c0376aa4ebd8f2571f4b1f45c83cb3
include/mesos/v1/resources.hpp a6216858136c82d5ebb5da3bb22c0b8a58a3b75d
src/common/resources.cpp 7ee4dae1389e037531aec533a3d235ee06443ea8
src/examples/persistent_volume_framework.cpp 1f8f4bea7ce4d1a919034e0639b4f70e7c9961cf
src/master/allocator/mesos/hierarchical.cpp 5b6efe5faa3c3b10f1f714f582a155b368f8ccaf
src/v1/resources.cpp 5c0a196cf87b377b345e53a36ed24727d52381ca
Diff: https://reviews.apache.org/r/63312/diff/2/
Changes: https://reviews.apache.org/r/63312/diff/1-2/
Testing
-------
make check
Thanks,
Jie Yu