You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Armand Grillet <ag...@mesosphere.io> on 2017/07/31 13:01:44 UTC
Re: Review Request 61112: Added uris flag to balloon framework.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61112/
-----------------------------------------------------------
(Updated July 31, 2017, 1:01 p.m.)
Review request for mesos, Alexander Rukletsov and Alexander Rojas.
Changes
-------
Suggestion applied.
Summary (updated)
-----------------
Added uris flag to balloon framework.
Bugs: MESOS-7814
https://issues.apache.org/jira/browse/MESOS-7814
Repository: mesos
Description (updated)
-------
Allows to set URIs that will be fetched before running the executor.
Diffs (updated)
-----
src/examples/balloon_framework.cpp b8c8e387bd8592bc1d27ff62fa9fd5397ad71fb9
Diff: https://reviews.apache.org/r/61112/diff/2/
Changes: https://reviews.apache.org/r/61112/diff/1-2/
Testing
-------
$ make check
Thanks,
Armand Grillet
Re: Review Request 61112: Added executor_uris flag to long lived and
balloon frameworks.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61112/#review181892
-----------------------------------------------------------
Fix it, then Ship it!
I will fix the outstanding issues and commit this for you.
src/examples/balloon_framework.cpp
Lines 46 (patched)
<https://reviews.apache.org/r/61112/#comment257681>
Looks like we should `#include <stout/protobuf.hpp>` instead of this file.
src/examples/balloon_framework.cpp
Line 109 (original), 114 (patched)
<https://reviews.apache.org/r/61112/#comment257682>
Let's mention this flag is deprecated.
src/examples/balloon_framework.cpp
Lines 118-124 (patched)
<https://reviews.apache.org/r/61112/#comment257684>
For consistency, with other JSON flags, how about this:
```
"The value could be a JSON-formatted string of `URI`s that\n"
"should be fetched before running the executor, or a file\n"
"path containing the JSON-formatted `URI`s. Path must be of\n"
"the form `file:///path/to/file` or `/path/to/file`.\n"
"This flag replaces `--executor_uri`.\n"
"See the `CommandInfo::URI` message in `mesos.proto` for the\n"
"expected format.\n"
```
src/examples/balloon_framework.cpp
Lines 133-135 (original), 159-161 (patched)
<https://reviews.apache.org/r/61112/#comment257685>
Add a TODO for deprecation:
```
// TODO(armand): Remove the `--executor_uri` flag after the
// deprecation cycle, started in 1.4.0.
```
src/examples/balloon_framework.cpp
Line 500 (original), 533-534 (patched)
<https://reviews.apache.org/r/61112/#comment257686>
Blank line, please.
- Alexander Rukletsov
On Aug. 1, 2017, 10:20 a.m., Armand Grillet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61112/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2017, 10:20 a.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Till Toenshoff.
>
>
> Bugs: MESOS-7814
> https://issues.apache.org/jira/browse/MESOS-7814
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allows to set URIs that will be fetched before running the executor.
>
>
> Diffs
> -----
>
> src/examples/balloon_framework.cpp b8c8e387bd8592bc1d27ff62fa9fd5397ad71fb9
> src/examples/long_lived_framework.cpp af5b43ee6994cf703f412436d6d561d9abd3b26d
>
>
> Diff: https://reviews.apache.org/r/61112/diff/3/
>
>
> Testing
> -------
>
> $ make check
>
>
> Thanks,
>
> Armand Grillet
>
>
Re: Review Request 61112: Added executor_uris flag to long lived and
balloon frameworks.
Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61112/
-----------------------------------------------------------
(Updated Aug. 1, 2017, 10:20 a.m.)
Review request for mesos, Alexander Rukletsov and Till Toenshoff.
Bugs: MESOS-7814
https://issues.apache.org/jira/browse/MESOS-7814
Repository: mesos
Description
-------
Allows to set URIs that will be fetched before running the executor.
Diffs
-----
src/examples/balloon_framework.cpp b8c8e387bd8592bc1d27ff62fa9fd5397ad71fb9
src/examples/long_lived_framework.cpp af5b43ee6994cf703f412436d6d561d9abd3b26d
Diff: https://reviews.apache.org/r/61112/diff/3/
Testing
-------
$ make check
Thanks,
Armand Grillet
Re: Review Request 61112: Added executor_uris flag to long lived and
balloon frameworks.
Posted by Armand Grillet <ag...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61112/
-----------------------------------------------------------
(Updated July 31, 2017, 9:39 p.m.)
Review request for mesos, Alexander Rukletsov and Till Toenshoff.
Changes
-------
Rsolved issues and flag added to long lived framework.
Summary (updated)
-----------------
Added executor_uris flag to long lived and balloon frameworks.
Bugs: MESOS-7814
https://issues.apache.org/jira/browse/MESOS-7814
Repository: mesos
Description
-------
Allows to set URIs that will be fetched before running the executor.
Diffs (updated)
-----
src/examples/balloon_framework.cpp b8c8e387bd8592bc1d27ff62fa9fd5397ad71fb9
src/examples/long_lived_framework.cpp af5b43ee6994cf703f412436d6d561d9abd3b26d
Diff: https://reviews.apache.org/r/61112/diff/3/
Changes: https://reviews.apache.org/r/61112/diff/2-3/
Testing
-------
$ make check
Thanks,
Armand Grillet
Re: Review Request 61112: Added uris flag to balloon framework.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61112/#review181799
-----------------------------------------------------------
src/examples/balloon_framework.cpp
Lines 20 (patched)
<https://reviews.apache.org/r/61112/#comment257552>
Do you still need this?
src/examples/balloon_framework.cpp
Lines 26 (patched)
<https://reviews.apache.org/r/61112/#comment257553>
Why do you need this one?
src/examples/balloon_framework.cpp
Lines 55 (patched)
<https://reviews.apache.org/r/61112/#comment257554>
Don't need this anymore?
src/examples/balloon_framework.cpp
Lines 127 (patched)
<https://reviews.apache.org/r/61112/#comment257548>
Let's call it `--executor_uris` for calrity.
src/examples/balloon_framework.cpp
Lines 132 (patched)
<https://reviews.apache.org/r/61112/#comment257555>
s\URI\CommandInfo::URI because `URI` is not a top level message.
src/examples/balloon_framework.cpp
Lines 498-504 (original), 529-551 (patched)
<https://reviews.apache.org/r/61112/#comment257549>
I'd suggest to use either `--executor_uri` or new `--uris` flag. Let's update comments for both flags to mention this, add a `CHECK` above that only a single one is set, and leave a deprecation `TODO` for the former flag.
src/examples/balloon_framework.cpp
Lines 540 (patched)
<https://reviews.apache.org/r/61112/#comment257550>
Indent is off.
src/examples/balloon_framework.cpp
Lines 543-545 (patched)
<https://reviews.apache.org/r/61112/#comment257556>
`EXIT(EXIT_FAILURE)` instead?
src/examples/balloon_framework.cpp
Lines 548-550 (patched)
<https://reviews.apache.org/r/61112/#comment257557>
Can we use `CopyFrom` from `RepeatedPtrField` here?
- Alexander Rukletsov
On July 31, 2017, 1:01 p.m., Armand Grillet wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61112/
> -----------------------------------------------------------
>
> (Updated July 31, 2017, 1:01 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov and Alexander Rojas.
>
>
> Bugs: MESOS-7814
> https://issues.apache.org/jira/browse/MESOS-7814
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Allows to set URIs that will be fetched before running the executor.
>
>
> Diffs
> -----
>
> src/examples/balloon_framework.cpp b8c8e387bd8592bc1d27ff62fa9fd5397ad71fb9
>
>
> Diff: https://reviews.apache.org/r/61112/diff/2/
>
>
> Testing
> -------
>
> $ make check
>
>
> Thanks,
>
> Armand Grillet
>
>