You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Timothy Chen <tn...@apache.org> on 2014/08/30 01:04:18 UTC
Review Request 25205: Fix command executor path check
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25205/
-----------------------------------------------------------
Review request for mesos and Benjamin Hindman.
Repository: mesos-git
Description
-------
Fix command executor path check.
Currently we expand the symlinks when we assign the command executor path when we write the ExecutorInfo's command value, but we don't do the same when we perform a string match later to check if the Executor is a commandExecutor.
This patches fixes so it does both on both place.
Diffs
-----
src/slave/slave.hpp 9d4607ef126f40ade9c861e3ea0eb41f10a3dff9
src/slave/slave.cpp 5c76dd1b9d3f7d262053aa4c20ebc2e8a00a0f4e
Diff: https://reviews.apache.org/r/25205/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 25205: Fix command executor path check
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25205/#review51950
-----------------------------------------------------------
Bad patch!
Reviews applied: [25205]
Failed command: ./support/mesos-style.py
Error:
Checking 504 files using filter --filter=-,+build/class,+build/deprecated,+build/endif_comment,+readability/todo,+readability/namespace,+runtime/vlog,+whitespace/blank_line,+whitespace/comma,+whitespace/ending_newline,+whitespace/forcolon,+whitespace/indent,+whitespace/line_length,+whitespace/tab,+whitespace/todo
src/slave/slave.cpp:2681: Tab found; better to use spaces [whitespace/tab] [1]
Total errors found: 1
- Mesos ReviewBot
On Aug. 29, 2014, 11:04 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25205/
> -----------------------------------------------------------
>
> (Updated Aug. 29, 2014, 11:04 p.m.)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Fix command executor path check.
>
> Currently we expand the symlinks when we assign the command executor path when we write the ExecutorInfo's command value, but we don't do the same when we perform a string match later to check if the Executor is a commandExecutor.
>
> This patches fixes so it does both on both place.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 9d4607ef126f40ade9c861e3ea0eb41f10a3dff9
> src/slave/slave.cpp 5c76dd1b9d3f7d262053aa4c20ebc2e8a00a0f4e
>
> Diff: https://reviews.apache.org/r/25205/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 25205: Fix command executor path check
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25205/#review52143
-----------------------------------------------------------
Patch looks great!
Reviews applied: [25205]
All tests passed.
- Mesos ReviewBot
On Sept. 2, 2014, 8:43 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25205/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2014, 8:43 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Fix command executor path check.
>
>
> Currently we expand the symlinks when we assign the command executor path when we write the ExecutorInfo's command value, but we don't do the same when we perform a string match later to check if the Executor is a commandExecutor.
>
>
> This patches fixes so it does both on both place.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 9d4607ef126f40ade9c861e3ea0eb41f10a3dff9
> src/slave/slave.cpp 5c76dd1b9d3f7d262053aa4c20ebc2e8a00a0f4e
>
> Diff: https://reviews.apache.org/r/25205/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 25205: Fix command executor path check
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25205/#review52132
-----------------------------------------------------------
Ship it!
Magnificent!
src/slave/slave.hpp
<https://reviews.apache.org/r/25205/#comment90878>
s/an/a/
src/slave/slave.cpp
<https://reviews.apache.org/r/25205/#comment90879>
Can you wrap after the '='?
- Adam B
On Sept. 2, 2014, 1:43 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25205/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2014, 1:43 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Fix command executor path check.
>
>
> Currently we expand the symlinks when we assign the command executor path when we write the ExecutorInfo's command value, but we don't do the same when we perform a string match later to check if the Executor is a commandExecutor.
>
>
> This patches fixes so it does both on both place.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 9d4607ef126f40ade9c861e3ea0eb41f10a3dff9
> src/slave/slave.cpp 5c76dd1b9d3f7d262053aa4c20ebc2e8a00a0f4e
>
> Diff: https://reviews.apache.org/r/25205/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 25205: Fix command executor path check
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25205/#review52533
-----------------------------------------------------------
Patch looks great!
Reviews applied: [25205]
All tests passed.
- Mesos ReviewBot
On Sept. 5, 2014, 4:34 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25205/
> -----------------------------------------------------------
>
> (Updated Sept. 5, 2014, 4:34 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Review: https://reviews.apache.org/r/25205
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 062e961a0ff2cfa88057a0d01114375cb86ffd24
> src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525
>
> Diff: https://reviews.apache.org/r/25205/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 25205: Fix command executor path check
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25205/
-----------------------------------------------------------
(Updated Sept. 5, 2014, 4:34 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen.
Repository: mesos-git
Description (updated)
-------
Review: https://reviews.apache.org/r/25205
Diffs (updated)
-----
src/slave/slave.hpp 062e961a0ff2cfa88057a0d01114375cb86ffd24
src/slave/slave.cpp bd31831022c97e68b0293d66e1eb5f28ac508525
Diff: https://reviews.apache.org/r/25205/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 25205: Fix command executor path check
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25205/#review52333
-----------------------------------------------------------
Bad patch!
Reviews applied: [25205]
Failed command: git apply --index 25205.patch
Error:
error: patch failed: src/slave/slave.hpp:522
error: src/slave/slave.hpp: patch does not apply
- Mesos ReviewBot
On Sept. 3, 2014, 8:30 p.m., Timothy Chen wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25205/
> -----------------------------------------------------------
>
> (Updated Sept. 3, 2014, 8:30 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Fix command executor path check.
>
>
> Currently we expand the symlinks when we assign the command executor path when we write the ExecutorInfo's command value, but we don't do the same when we perform a string match later to check if the Executor is a commandExecutor.
>
>
> This patches fixes so it does both on both place.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 9d4607e
> src/slave/slave.cpp 5c76dd1
>
> Diff: https://reviews.apache.org/r/25205/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Timothy Chen
>
>
Re: Review Request 25205: Fix command executor path check
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25205/
-----------------------------------------------------------
(Updated Sept. 3, 2014, 8:30 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen.
Repository: mesos-git
Description
-------
Fix command executor path check.
Currently we expand the symlinks when we assign the command executor path when we write the ExecutorInfo's command value, but we don't do the same when we perform a string match later to check if the Executor is a commandExecutor.
This patches fixes so it does both on both place.
Diffs (updated)
-----
src/slave/slave.hpp 9d4607e
src/slave/slave.cpp 5c76dd1
Diff: https://reviews.apache.org/r/25205/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 25205: Fix command executor path check
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25205/
-----------------------------------------------------------
(Updated Sept. 2, 2014, 8:43 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen.
Repository: mesos-git
Description (updated)
-------
Fix command executor path check.
Currently we expand the symlinks when we assign the command executor path when we write the ExecutorInfo's command value, but we don't do the same when we perform a string match later to check if the Executor is a commandExecutor.
This patches fixes so it does both on both place.
Diffs
-----
src/slave/slave.hpp 9d4607ef126f40ade9c861e3ea0eb41f10a3dff9
src/slave/slave.cpp 5c76dd1b9d3f7d262053aa4c20ebc2e8a00a0f4e
Diff: https://reviews.apache.org/r/25205/diff/
Testing
-------
make check
Thanks,
Timothy Chen
Re: Review Request 25205: Fix command executor path check
Posted by Timothy Chen <tn...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25205/
-----------------------------------------------------------
(Updated Sept. 2, 2014, 8:43 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, and Niklas Nielsen.
Repository: mesos-git
Description (updated)
-------
Review: https://reviews.apache.org/r/25205
Diffs (updated)
-----
src/slave/slave.hpp 9d4607ef126f40ade9c861e3ea0eb41f10a3dff9
src/slave/slave.cpp 5c76dd1b9d3f7d262053aa4c20ebc2e8a00a0f4e
Diff: https://reviews.apache.org/r/25205/diff/
Testing
-------
make check
Thanks,
Timothy Chen