Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.

[Guangya Liu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39388/#review103079
---

Ship it!


Ship It!

- Guangya Liu


On 十月 17, 2015, 11:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39388/
> ---
> 
> (Updated 十月 17, 2015, 11:18 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3740
> https://issues.apache.org/jira/browse/MESOS-3740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
> 
> Diff: https://reviews.apache.org/r/39388/diff/
> 
> 
> Testing
> ---
> 
> Added `DockerContainerizerTest.ROOT_DOCKER_LaunchWithLibprocessIP` test which 
> fails without the changes made to `src/docker/docker.cpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.

[Michael Park]

> On Oct. 18, 2015, 4:10 a.m., haosdent huang wrote:
> > src/docker/docker.cpp, line 430
> > 
> >
> > Seems we already have this env in 
> > https://github.com/apache/mesos/blob/master/src/slave/containerizer/containerizer.cpp#L254
> 
> Klaus Ma wrote:
> I think those environment are used for `docker run` instead of passing 
> them to the container.

That one is used to pass the `LIBPROCES_IP` to the `subprocess` call when we're 
launching a process, whereas this is used to pass the `LIBPROCESS_IP` to 
`docker->run` when we're launching a container via `-e`.


- Michael


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39388/#review103050
---


On Oct. 17, 2015, 11:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39388/
> ---
> 
> (Updated Oct. 17, 2015, 11:18 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3740
> https://issues.apache.org/jira/browse/MESOS-3740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
> 
> Diff: https://reviews.apache.org/r/39388/diff/
> 
> 
> Testing
> ---
> 
> Added `DockerContainerizerTest.ROOT_DOCKER_LaunchWithLibprocessIP` test which 
> fails without the changes made to `src/docker/docker.cpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 38578: Update upgrade.md for SUPPRESS related upgrade

[Guangya Liu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38578/#review103087
---


Vinod, can you please help review this again? Thanks.

- Guangya Liu


On 十月 13, 2015, 1:08 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38578/
> ---
> 
> (Updated 十月 13, 2015, 1:08 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3037
> https://issues.apache.org/jira/browse/MESOS-3037
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update upgrade.md for SUPPRESS related upgrade
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md c2e39b7ddb12a00cd7ce5ca78bb34af9ba67b3c0 
> 
> Diff: https://reviews.apache.org/r/38578/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 39420: Added '--parent' option and made apply-review.sh call apply-reviews.py.

[Artem Harutyunyan]

> On Oct. 18, 2015, 7:14 a.m., Guangya Liu wrote:
> > Why not make this as a default option? I think that we need to apply the 
> > whole chain of patch for development, thoughts? Thanks.

It will break the verification script that ASF Jenkins currently uses.


- Artem


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39420/#review103056
---


On Oct. 17, 2015, 7:43 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39420/
> ---
> 
> (Updated Oct. 17, 2015, 7:43 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, Marco Massenzio, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added '--parent' option and made apply-review.sh call apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-review.sh 6391451542e9e8847ec38e2ad9d9acf552afead3 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39420/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7.
> 
> - with and without '-p'.
> - Tested reviews with and without parents.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39262: Windows:[1/3] Moved `os::environ` -> `os::raw::environment`.

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39262/#review103064
---

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/os/raw/environment.hpp (line 
39)


missing space


- Joris Van Remoortere


On Oct. 18, 2015, 6:40 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39262/
> ---
> 
> (Updated Oct. 18, 2015, 6:40 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows standard headers define a macro, `environ`, that holds the
> environment data for the executing process. Unfortunately, the existance
> of this macro makes it impossible to use a function called
> `os::environ`.
> 
> Our solution to this problem in this commit is to move the function
> family at `os::environ`* to `os::raw::environment`. This family exists
> in the `os::raw` because they are the "unstructured" variants of
> functions with the same names that exist in the `os` namespace. For
> example, `os::raw::environment` returns a `char**` which is the
> "unstructured" equivalent of `os::environment`, which returns a
> `map`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5141c1369af60afd6cd5c70c6295d575ea960a83 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/raw/environment.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> a042d061159c83e1652e7288b90809981d2818c9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 2b0966889af73238a08e29f1136d0ce286a0ffda 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> ee2a7a79617612ed448e650cc77608c4962fe5a5 
> 
> Diff: https://reviews.apache.org/r/39262/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Ubuntu 15, OS X 10.10, and running the `check` project on 
> Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 31625: Fixed broken link in framework development guide

[Neil Conway]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31625/#review103073
---


This seems like the wrong way to fix the problem: the core issue is that the 
markdown link conversion regexp is not correct. See 
https://reviews.apache.org/r/39385/ for a fix.

- Neil Conway


On March 2, 2015, 3:36 p.m., Palak Choudhary wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31625/
> ---
> 
> (Updated March 2, 2015, 3:36 p.m.)
> 
> 
> Review request for mesos and Dave Lester.
> 
> 
> Bugs: MESOS-2004
> https://issues.apache.org/jira/browse/MESOS-2004
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed broken link in framework development guide
> 
> 
> Diffs
> -
> 
>   docs/mesos-architecture.md 439a56d0aca5935a2c9f05e74908eda88e284297 
> 
> Diff: https://reviews.apache.org/r/31625/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Palak Choudhary
> 
>

Re: Review Request 39423: Fixed `ExamplesTest.PersistentVolumeFramework` on OS X El Capitan.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39423/#review103072
---


Patch looks great!

Reviews applied: [39423]

All tests passed.

- Mesos ReviewBot


On Oct. 18, 2015, 7:35 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39423/
> ---
> 
> (Updated Oct. 18, 2015, 7:35 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-3604
> https://issues.apache.org/jira/browse/MESOS-3604
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `MESOS_LAUNCHER_DIR` was being set to `/src/.libs` rather than 
> `/src`. This leads to us attemping to execute 
> `/src/.libs/mesos-executor` which doesn't have the correct 
> `DYLD_LIBRARY_PATH` set.
> 
> 
> Diffs
> -
> 
>   src/examples/persistent_volume_framework.cpp 
> 426d33813f8f17397c6ed29b8a8b33df4137bba6 
> 
> Diff: https://reviews.apache.org/r/39423/diff/
> 
> 
> Testing
> ---
> 
> ```
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from ExamplesTest
> [ RUN  ] ExamplesTest.PersistentVolumeFramework
> [   OK ] ExamplesTest.PersistentVolumeFramework (3391 ms)
> [--] 1 test from ExamplesTest (3391 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (3403 ms total)
> [  PASSED  ] 1 test.
> ```
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 39018: Added JSON parsing for Resources.

[Michael Park]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39018/#review103071
---



include/mesos/resources.hpp (line 37)


I think we can remove this now that the parts that require it were moved to 
`src/common/resources.cpp`, right?



include/mesos/v1/resources.hpp (line 36)


I think we can remove this now that the parts that require it were moved to 
`src/v1/resources.cpp`, right?


- Michael Park


On Oct. 14, 2015, 3:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 14, 2015, 3:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review 
> (https://reviews.apache.org/r/39102/).
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass, with one notable 
> failure (ResourcesTest.ParsingFromJSONError) related to a picojson issue 
> tracked here: https://issues.apache.org/jira/browse/MESOS-3698
> 
> The original resources parsing format is used throughout many other tests 
> (check `src/tests/sorter_tests.cpp`, for example), so it's clear that the 
> original parsing continues to work correctly.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39420: Added '--parent' option and made apply-review.sh call apply-reviews.py.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39420/#review103080
---


Patch looks great!

Reviews applied: [38705, 38883, 39410, 39420]

All tests passed.

- Mesos ReviewBot


On Oct. 18, 2015, 10:33 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39420/
> ---
> 
> (Updated Oct. 18, 2015, 10:33 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, Marco Massenzio, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added '--parent' option and made apply-review.sh call apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-review.sh 6391451542e9e8847ec38e2ad9d9acf552afead3 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39420/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7.
> 
> - with and without '-p'.
> - Tested reviews with and without parents.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39388: Explicitly set the `LIBPROCESS_IP` env variable for docker containers.

[haosdent huang]

> On Oct. 18, 2015, 4:10 a.m., haosdent huang wrote:
> > src/docker/docker.cpp, line 430
> > 
> >
> > Seems we already have this env in 
> > https://github.com/apache/mesos/blob/master/src/slave/containerizer/containerizer.cpp#L254
> 
> Klaus Ma wrote:
> I think those environment are used for `docker run` instead of passing 
> them to the container.
> 
> Michael Park wrote:
> That one is used to pass the `LIBPROCES_IP` to the `subprocess` call when 
> we're launching a process, whereas this is used to pass the `LIBPROCESS_IP` 
> to `docker->run` when we're launching a container via `-e`.

Thank you very much for explanation, got it!


- haosdent


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39388/#review103050
---


On Oct. 17, 2015, 11:18 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39388/
> ---
> 
> (Updated Oct. 17, 2015, 11:18 p.m.)
> 
> 
> Review request for mesos and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3740
> https://issues.apache.org/jira/browse/MESOS-3740
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.cpp 56d63dc75637c9f89a239af371f476a85a570696 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4bb65afd0ee61cafef68e064a697fdce65d60058 
> 
> Diff: https://reviews.apache.org/r/39388/diff/
> 
> 
> Testing
> ---
> 
> Added `DockerContainerizerTest.ROOT_DOCKER_LaunchWithLibprocessIP` test which 
> fails without the changes made to `src/docker/docker.cpp`.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 39386: Fix uncorrect launcher dir in docker executor.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39386/#review103090
---


Patch looks great!

Reviews applied: [39386]

All tests passed.

- Mesos ReviewBot


On Oct. 19, 2015, 2:59 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39386/
> ---
> 
> (Updated Oct. 19, 2015, 2:59 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3738
> https://issues.apache.org/jira/browse/MESOS-3738
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix uncorrect launcher dir in docker executor.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6 
>   src/docker/executor.hpp 8ab4e98c41017e8420b98c63a59d8e4bc1a99172 
>   src/docker/executor.cpp 1e4901335854c49e46cd7b132e79ccb11cd72ade 
>   src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 
>   src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2 
> 
> Diff: https://reviews.apache.org/r/39386/diff/
> 
> 
> Testing
> ---
> 
> * make check
> * make install and then test with marathon to check if launcher_dir passes 
> correctly.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 39386: Fix uncorrect launcher dir in docker executor.

[haosdent huang]

> On Oct. 16, 2015, 11:34 p.m., Niklas Nielsen wrote:
> > src/docker/executor.cpp, line 581
> > 
> >
> > What happens when you remove this fallback, taken a user don't provide 
> > the launcher dir?

Hi, @nnielsen. Do you mean when user don't provide `launcher_dir` here, what it 
would happend? `launcher_dir` is a require option here, if user don't provide 
it, the executor would exit.


- haosdent


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39386/#review103005
---


On Oct. 19, 2015, 2:59 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39386/
> ---
> 
> (Updated Oct. 19, 2015, 2:59 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3738
> https://issues.apache.org/jira/browse/MESOS-3738
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix uncorrect launcher dir in docker executor.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6 
>   src/docker/executor.hpp 8ab4e98c41017e8420b98c63a59d8e4bc1a99172 
>   src/docker/executor.cpp 1e4901335854c49e46cd7b132e79ccb11cd72ade 
>   src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 
>   src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2 
> 
> Diff: https://reviews.apache.org/r/39386/diff/
> 
> 
> Testing
> ---
> 
> * make check
> * make install and then test with marathon to check if launcher_dir passes 
> correctly.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 39420: Added '--parent' option and made apply-review.sh call apply-reviews.py.

[Artem Harutyunyan]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39420/
---

(Updated Oct. 18, 2015, 3:33 p.m.)


Review request for mesos, Joris Van Remoortere, Joseph Wu, Marco Massenzio, and 
Vinod Kone.


Changes
---

space/tab cleanup.


Bugs: MESOS-3468
https://issues.apache.org/jira/browse/MESOS-3468


Repository: mesos


Description
---

Added '--parent' option and made apply-review.sh call apply-reviews.py.


Diffs (updated)
-

  support/apply-review.sh 6391451542e9e8847ec38e2ad9d9acf552afead3 
  support/apply-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/39420/diff/


Testing
---

Tested with python 2.7.

- with and without '-p'.
- Tested reviews with and without parents.


Thanks,

Artem Harutyunyan

Re: Review Request 39420: Added '--parent' option and made apply-review.sh call apply-reviews.py.

[Artem Harutyunyan]

> On Oct. 18, 2015, 7:14 a.m., Guangya Liu wrote:
> > support/apply-reviews.py, lines 272-274
> > 
> >
> > Tab or blank space? If Tab, we should switch to blank space

Thanks for spotting this! My editor was sneaking tabs in this whole time. I 
went back and cleaned previous reviews as well.


- Artem


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39420/#review103056
---


On Oct. 18, 2015, 3:33 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39420/
> ---
> 
> (Updated Oct. 18, 2015, 3:33 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, Marco Massenzio, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added '--parent' option and made apply-review.sh call apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-review.sh 6391451542e9e8847ec38e2ad9d9acf552afead3 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39420/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7.
> 
> - with and without '-p'.
> - Tested reviews with and without parents.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39420: Added '--parent' option and made apply-review.sh call apply-reviews.py.

[Guangya Liu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39420/#review103082
---

Ship it!


Ship It!

- Guangya Liu


On 十月 18, 2015, 10:33 p.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39420/
> ---
> 
> (Updated 十月 18, 2015, 10:33 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, Marco Massenzio, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added '--parent' option and made apply-review.sh call apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-review.sh 6391451542e9e8847ec38e2ad9d9acf552afead3 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39420/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7.
> 
> - with and without '-p'.
> - Tested reviews with and without parents.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39386: Fix uncorrect launcher dir in docker executor.

[haosdent huang]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39386/
---

(Updated Oct. 19, 2015, 2:59 a.m.)


Review request for mesos and Timothy Chen.


Changes
---

Update configuration document.


Bugs: MESOS-3738
https://issues.apache.org/jira/browse/MESOS-3738


Repository: mesos


Description
---

Fix uncorrect launcher dir in docker executor.


Diffs (updated)
-

  docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6 
  src/docker/executor.hpp 8ab4e98c41017e8420b98c63a59d8e4bc1a99172 
  src/docker/executor.cpp 1e4901335854c49e46cd7b132e79ccb11cd72ade 
  src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 
  src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2 

Diff: https://reviews.apache.org/r/39386/diff/


Testing
---

* make check
* make install and then test with marathon to check if launcher_dir passes 
correctly.


Thanks,

haosdent huang

Re: Review Request 39386: Fix uncorrect launcher dir in docker executor.

[haosdent huang]

> On Oct. 16, 2015, 5:30 p.m., Marco Massenzio wrote:
> > Thanks for doing this.
> > 
> > I think you will need to document the flags' usage in `configuration.md` 
> > (or wherever appropriate) and state clearly that it's **required** and what 
> > it should point to (in other words, what binary(ies) will we be looking 
> > for?) - or users will be confused by the error message.
> 
> Marco Massenzio wrote:
> Actually - would it be possible to add a unit test to catch the scenario 
> that was reported in the Jira and make sure this fixes it?
> (when it comes to interaction between OS env vars and flags, it's always 
> tricky)

Because we pass the launcher_dir flag directly instead of get it from argv. If 
the HealthCheckTest.ROOT_DOCKER_DockerHealthyTask and 
HealthCheckTest.ROOT_DOCKER_DockerHealthStatusChange could pass, it show the 
launcher_dir is correct. So I don't add new test cases here.


- haosdent


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39386/#review102939
---


On Oct. 19, 2015, 2:59 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39386/
> ---
> 
> (Updated Oct. 19, 2015, 2:59 a.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-3738
> https://issues.apache.org/jira/browse/MESOS-3738
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix uncorrect launcher dir in docker executor.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 9443d5fc2de0f46f9e117b08e29b09ff3a4579c6 
>   src/docker/executor.hpp 8ab4e98c41017e8420b98c63a59d8e4bc1a99172 
>   src/docker/executor.cpp 1e4901335854c49e46cd7b132e79ccb11cd72ade 
>   src/slave/containerizer/docker.cpp 702295808475c092dff66417f42af89b90e6d50d 
>   src/slave/flags.cpp 1bf394ea62fde29caa6705cd5d156eae452adbf2 
> 
> Diff: https://reviews.apache.org/r/39386/diff/
> 
> 
> Testing
> ---
> 
> * make check
> * make install and then test with marathon to check if launcher_dir passes 
> correctly.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 38883: Removed calls to apply-review.sh script. Added support for amending commit messages.

[Artem Harutyunyan]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38883/
---

(Updated Oct. 18, 2015, 3:31 p.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


Changes
---

space/tab cleanup.


Bugs: MESOS-3468
https://issues.apache.org/jira/browse/MESOS-3468


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/38883/diff/


Testing
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 39410: Added support for github to apply-reviews.py.

[Artem Harutyunyan]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39410/
---

(Updated Oct. 18, 2015, 3:32 p.m.)


Review request for mesos, Adam B, Joris Van Remoortere, Joseph Wu, Marco 
Massenzio, and Vinod Kone.


Changes
---

space/tab cleanup.


Bugs: MESOS-3468
https://issues.apache.org/jira/browse/MESOS-3468


Repository: mesos


Description
---

Added support for github to apply-reviews.py.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/39410/diff/


Testing
---

Tested with python 2.7


Thanks,

Artem Harutyunyan

Re: Review Request 38705: Added support for applying a review chain (apply-reviews.py).

[Artem Harutyunyan]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38705/
---

(Updated Oct. 18, 2015, 3:30 p.m.)


Review request for mesos, Benjamin Hindman, Joris Van Remoortere, Joseph Wu, 
and Vinod Kone.


Changes
---

space/tab cleanup.


Bugs: MESOS-3468
https://issues.apache.org/jira/browse/MESOS-3468


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  support/apply-reviews.py PRE-CREATION 

Diff: https://reviews.apache.org/r/38705/diff/


Testing
---

Tested the script with python 2.7.


Thanks,

Artem Harutyunyan

Re: Review Request 39330: libprocess: Replaced usage of "volatile" with std::atomic.

[Neil Conway]

> On Oct. 17, 2015, 8:43 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/include/process/owned.hpp, line 82
> > 
> >
> > Wish we could use `nullptr`, but we currently only allow `NULL` for now.
> > Here and below.

Sure, easy to fix. I'm curious -- why is nullptr disallowed? (It has been 
supported since GCC 4.6...)


- Neil


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39330/#review103028
---


On Oct. 14, 2015, 10:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39330/
> ---
> 
> (Updated Oct. 14, 2015, 10:29 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3326
> https://issues.apache.org/jira/browse/MESOS-3326
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See MESOS-3326. We adopted std::atomic in most of the code base earlier 
> (commits
> 87de003c6e8a, 4b938052b6af, and 4a01850c5540), but a few places were omitted;
> those locations are fixed by this commit.
> 
> There's one remaining place to improve: we use the GCC intrinsic
> __sync_synchronize() in 3rdparty/libprocess/include/process/logging.h. Because
> that is used to protect modifications to the FLAGS_v variable defined by glog,
> we can't easily adapt it to use std::atomic.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/owned.hpp 
> bc5b527152c8864544ad58070c0bfc81639056da 
>   3rdparty/libprocess/include/process/shared.hpp 
> 021807b961bb55f11c9e04327135bd83f4d86c21 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> e5277de5b5bdea4a44606cda7fbf69a559aeebbe 
> 
> Diff: https://reviews.apache.org/r/39330/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39429: Replaced volatile, GCC intrinsics with std::atomic.

[Neil Conway]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39429/
---

(Updated Oct. 19, 2015, 4:48 a.m.)


Review request for mesos and Joris Van Remoortere.


Bugs: MESOS-3326
https://issues.apache.org/jira/browse/MESOS-3326


Repository: mesos


Description
---

See MESOS-3326. We adopted std::atomic in most of the code base earlier (commits
87de003c6e8a, 4b938052b6af, and 4a01850c5540), but a few places were omitted;
those locations are fixed by this commit.

There's one last place to fix: we use the GCC intrinsic __sync_synchronize() in
3rdparty/libprocess/include/process/logging.h. Because that is used to protect
modifications to the FLAGS_v variable defined by glog, we can't easily adapt it
to use std::atomic.


Diffs (updated)
-

  3rdparty/libprocess/include/process/owned.hpp 
17c42614155fed8fec185f29f9a9babb71ff5f85 
  3rdparty/libprocess/include/process/shared.hpp 
021807b961bb55f11c9e04327135bd83f4d86c21 
  3rdparty/libprocess/src/tests/process_tests.cpp 
a3c57a261c09f7b76b78225719e64a26f2d66cd3 

Diff: https://reviews.apache.org/r/39429/diff/


Testing
---


Thanks,

Neil Conway

Re: Review Request 39428: Fixed typo in comment, minor style fixes.

[Guangya Liu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39428/#review103093
---

Ship it!


Ship It!

- Guangya Liu


On 十月 19, 2015, 4:20 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39428/
> ---
> 
> (Updated 十月 19, 2015, 4:20 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3326
> https://issues.apache.org/jira/browse/MESOS-3326
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in comment, minor style fixes.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/owned.hpp 
> bc5b527152c8864544ad58070c0bfc81639056da 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> e5277de5b5bdea4a44606cda7fbf69a559aeebbe 
> 
> Diff: https://reviews.apache.org/r/39428/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39230: Added prevention of SASL deprecation warnings all around its invocations on OS X.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39230/#review103099
---


Patch looks great!

Reviews applied: [39230]

All tests passed.

- Mesos ReviewBot


On Oct. 19, 2015, 4:27 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39230/
> ---
> 
> (Updated Oct. 19, 2015, 4:27 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3030
> https://issues.apache.org/jira/browse/MESOS-3030
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   configure.ac 66f9b32 
>   src/authentication/cram_md5/authenticatee.cpp b03b44a 
>   src/authentication/cram_md5/authenticator.cpp f238872 
>   src/authentication/cram_md5/auxprop.cpp abf0f8d 
> 
> Diff: https://reviews.apache.org/r/39230/diff/
> 
> 
> Testing
> ---
> 
> ./configure && make check 
> OSX, clang3.7 + gcc4.9
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 39429: Replaced volatile, GCC intrinsics with std::atomic.

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39429/#review103095
---


Bad patch!

Reviews applied: [39428, 39429]

Failed command: ./support/apply-review.sh -n -r 39429

Error:
 2015-10-19 04:39:57 URL:https://reviews.apache.org/r/39429/diff/raw/ 
[9602/9602] -> "39429.patch" [1]
error: patch failed: 3rdparty/libprocess/src/tests/process_tests.cpp:21
error: 3rdparty/libprocess/src/tests/process_tests.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Oct. 19, 2015, 4:20 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39429/
> ---
> 
> (Updated Oct. 19, 2015, 4:20 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3326
> https://issues.apache.org/jira/browse/MESOS-3326
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See MESOS-3326. We adopted std::atomic in most of the code base earlier 
> (commits
> 87de003c6e8a, 4b938052b6af, and 4a01850c5540), but a few places were omitted;
> those locations are fixed by this commit.
> 
> There's one last place to fix: we use the GCC intrinsic __sync_synchronize() 
> in
> 3rdparty/libprocess/include/process/logging.h. Because that is used to protect
> modifications to the FLAGS_v variable defined by glog, we can't easily adapt 
> it
> to use std::atomic.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/owned.hpp 
> bc5b527152c8864544ad58070c0bfc81639056da 
>   3rdparty/libprocess/include/process/shared.hpp 
> 021807b961bb55f11c9e04327135bd83f4d86c21 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> e5277de5b5bdea4a44606cda7fbf69a559aeebbe 
> 
> Diff: https://reviews.apache.org/r/39429/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 39429: Replaced volatile, GCC intrinsics with std::atomic.

[Neil Conway]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39429/
---

Review request for mesos and Joris Van Remoortere.


Bugs: MESOS-3326
https://issues.apache.org/jira/browse/MESOS-3326


Repository: mesos


Description
---

See MESOS-3326. We adopted std::atomic in most of the code base earlier (commits
87de003c6e8a, 4b938052b6af, and 4a01850c5540), but a few places were omitted;
those locations are fixed by this commit.

There's one last place to fix: we use the GCC intrinsic __sync_synchronize() in
3rdparty/libprocess/include/process/logging.h. Because that is used to protect
modifications to the FLAGS_v variable defined by glog, we can't easily adapt it
to use std::atomic.


Diffs
-

  3rdparty/libprocess/include/process/owned.hpp 
bc5b527152c8864544ad58070c0bfc81639056da 
  3rdparty/libprocess/include/process/shared.hpp 
021807b961bb55f11c9e04327135bd83f4d86c21 
  3rdparty/libprocess/src/tests/process_tests.cpp 
e5277de5b5bdea4a44606cda7fbf69a559aeebbe 

Diff: https://reviews.apache.org/r/39429/diff/


Testing
---


Thanks,

Neil Conway

Re: Review Request 39330: libprocess: Replaced usage of "volatile" with std::atomic.

[Neil Conway]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39330/#review103092
---


Seems like splitting the commit into two dependent commits (and rebasing) 
caused post-reviews to create a new review set -- sorry for the inconvenience. 
New reviews are here:

https://reviews.apache.org/r/39428/
https://reviews.apache.org/r/39429/

I'll mark this review discarded.

- Neil Conway


On Oct. 14, 2015, 10:29 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39330/
> ---
> 
> (Updated Oct. 14, 2015, 10:29 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3326
> https://issues.apache.org/jira/browse/MESOS-3326
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See MESOS-3326. We adopted std::atomic in most of the code base earlier 
> (commits
> 87de003c6e8a, 4b938052b6af, and 4a01850c5540), but a few places were omitted;
> those locations are fixed by this commit.
> 
> There's one remaining place to improve: we use the GCC intrinsic
> __sync_synchronize() in 3rdparty/libprocess/include/process/logging.h. Because
> that is used to protect modifications to the FLAGS_v variable defined by glog,
> we can't easily adapt it to use std::atomic.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/owned.hpp 
> bc5b527152c8864544ad58070c0bfc81639056da 
>   3rdparty/libprocess/include/process/shared.hpp 
> 021807b961bb55f11c9e04327135bd83f4d86c21 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> e5277de5b5bdea4a44606cda7fbf69a559aeebbe 
> 
> Diff: https://reviews.apache.org/r/39330/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 39230: Added prevention of SASL deprecation warnings all around its invocations on OS X.

[Till Toenshoff]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39230/
---

(Updated Oct. 19, 2015, 4:27 a.m.)


Review request for mesos and Michael Park.


Summary (updated)
-

Added prevention of SASL deprecation warnings all around its invocations on OS 
X.


Bugs: MESOS-3030
https://issues.apache.org/jira/browse/MESOS-3030


Repository: mesos


Description (updated)
---

see summary.


Diffs (updated)
-

  configure.ac 66f9b32 
  src/authentication/cram_md5/authenticatee.cpp b03b44a 
  src/authentication/cram_md5/authenticator.cpp f238872 
  src/authentication/cram_md5/auxprop.cpp abf0f8d 

Diff: https://reviews.apache.org/r/39230/diff/


Testing
---

./configure && make check 
OSX, clang3.7 + gcc4.9


Thanks,

Till Toenshoff

Review Request 39428: Fixed typo in comment, minor style fixes.

[Neil Conway]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39428/
---

Review request for mesos and Joris Van Remoortere.


Bugs: MESOS-3326
https://issues.apache.org/jira/browse/MESOS-3326


Repository: mesos


Description
---

Fixed typo in comment, minor style fixes.


Diffs
-

  3rdparty/libprocess/include/process/owned.hpp 
bc5b527152c8864544ad58070c0bfc81639056da 
  3rdparty/libprocess/src/tests/process_tests.cpp 
e5277de5b5bdea4a44606cda7fbf69a559aeebbe 

Diff: https://reviews.apache.org/r/39428/diff/


Testing
---


Thanks,

Neil Conway

Re: Review Request 39428: Fixed typo in comment, minor style fixes.

[Michael Park]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39428/#review103094
---

Ship it!


Ship It!

- Michael Park


On Oct. 19, 2015, 4:20 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39428/
> ---
> 
> (Updated Oct. 19, 2015, 4:20 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3326
> https://issues.apache.org/jira/browse/MESOS-3326
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in comment, minor style fixes.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/owned.hpp 
> bc5b527152c8864544ad58070c0bfc81639056da 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> e5277de5b5bdea4a44606cda7fbf69a559aeebbe 
> 
> Diff: https://reviews.apache.org/r/39428/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Klaus Ma]

> On Oct. 18, 2015, 11:02 p.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 764-768
> > 
> >
> > As mentioned before, let's remove the `default` case entirely. Refer to 
> > [MESOS-3754](https://issues.apache.org/jira/browse/MESOS-3754) for details.

I just remove `default:` becase keep `ABORT` for deprecated case 
(`google::protobuf::FieldDescriptor::TYPE_GROUP:`).


> On Oct. 18, 2015, 11:02 p.m., Michael Park wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp, lines 182-189
> > 
> >
> > Indent 2 spaces, not the strings themselves, but within the strings.

The 2 sparces indent are in the strings, do you mean we should move it out?


- Klaus


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38342/#review103078
---


On Oct. 16, 2015, 9:56 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> ---
> 
> (Updated Oct. 16, 2015, 9:56 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> ---
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 39265: Windows: Move `os::environment` to its own file.

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39265/#review103067
---

Ship it!



3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp (lines 92 - 93)


why is this pulled out by itself?



3rdparty/libprocess/3rdparty/stout/include/stout/os/environment.hpp (line 17)


`string`


- Joris Van Remoortere


On Oct. 13, 2015, 6:37 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39265/
> ---
> 
> (Updated Oct. 13, 2015, 6:37 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Move `os::environment` to its own file.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5141c1369af60afd6cd5c70c6295d575ea960a83 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/environment.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39265/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Ubuntu 15, OS X 10.10, and running the `check` project on 
> Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39264: Windows:[3/3] Transitioned Mesos tests to use `os::raw` family.

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39264/#review103066
---

Ship it!


- Joris Van Remoortere


On Oct. 13, 2015, 6:36 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39264/
> ---
> 
> (Updated Oct. 13, 2015, 6:36 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In the previous commits, we've added a family if wrapper functions in
> `os::raw` that allow us to get the environment variables of the
> executing process. This commit will transition the Mesos tests to use
> those functions.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp df33a6b7962f915df63c78eb6632b67d8874cef5 
> 
> Diff: https://reviews.apache.org/r/39264/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Ubuntu 15, OS X 10.10, and running the `check` project on 
> Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39263: Windows:[2/3] transitioned libprocess to use `os::raw::` family.

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39263/#review103065
---

Ship it!


- Joris Van Remoortere


On Oct. 13, 2015, 6:36 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39263/
> ---
> 
> (Updated Oct. 13, 2015, 6:36 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The last commit in this chain introduces a family of environment wrapper
> methods in the `os::raw` namespace. This commit will transition the
> process library to use these functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/subprocess.cpp 
> a457cbe35ad33531c49f74550cd570cf28758f5d 
> 
> Diff: https://reviews.apache.org/r/39263/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Ubuntu 15, OS X 10.10, and running the `check` project on 
> Windows 10.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38342: Add JSON::protobuf for google::protobuf::RepeatedPtrField (stout part)

[Michael Park]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38342/#review103078
---



3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp (lines 762 - 766)


As mentioned before, let's remove the `default` case entirely. Refer to 
[MESOS-3754](https://issues.apache.org/jira/browse/MESOS-3754) for details.



3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp (lines 182 - 189)


Indent 2 spaces, not the strings themselves, but within the strings.


- Michael Park


On Oct. 16, 2015, 9:56 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38342/
> ---
> 
> (Updated Oct. 16, 2015, 9:56 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Michael Park, and Jan Schlicht.
> 
> 
> Bugs: MESOS-3405
> https://issues.apache.org/jira/browse/MESOS-3405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `stout/protobuf.hpp` provides a `JSON::Protobuf` utility which 
> converts a `google::protobuf::Message` into a `JSON::Object`.
> We should add the support for `google::protobuf::RepeatedPtrField` by 
> introducing overloaded functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 2285ce9 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.cpp 68328a2 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.h 8ebb798 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.pb.cc 34eb6d0 
>   3rdparty/libprocess/3rdparty/stout/tests/protobuf_tests.proto 920f5c9 
> 
> Diff: https://reviews.apache.org/r/38342/diff/
> 
> 
> Testing
> ---
> 
> cd 3rdparty/libprocess/3rdparty/stout
> ./boostrap
> ./configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 39219: Windows: Added support for `slave/state.cpp`.

[Alex Clemmer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39219/
---

(Updated Oct. 18, 2015, 6:40 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Bugs: MESOS-3615, MESOS-3627, MESOS-3628, MESOS-3629, MESOS-3630, MESOS-3631, 
MESOS-3633, MESOS-3658, MESOS-3659, MESOS-3660, and MESOS-3693
https://issues.apache.org/jira/browse/MESOS-3615
https://issues.apache.org/jira/browse/MESOS-3627
https://issues.apache.org/jira/browse/MESOS-3628
https://issues.apache.org/jira/browse/MESOS-3629
https://issues.apache.org/jira/browse/MESOS-3630
https://issues.apache.org/jira/browse/MESOS-3631
https://issues.apache.org/jira/browse/MESOS-3633
https://issues.apache.org/jira/browse/MESOS-3658
https://issues.apache.org/jira/browse/MESOS-3659
https://issues.apache.org/jira/browse/MESOS-3660
https://issues.apache.org/jira/browse/MESOS-3693


Repository: mesos


Description
---

Windows: Added support for `slave/state.cpp`.


Diffs
-

  src/slave/state.hpp 5a1a9bb2c86639612a8f065b7a66c8179696297a 
  src/slave/state.cpp 81c4b96d879f974f0dfba3fb977184122eab 

Diff: https://reviews.apache.org/r/39219/diff/


Testing
---

Built CMake solution and ran `make check` (or equivalent) on Windows 10, Ubuntu 
15, OS X 10.10. Built autotools solution and ran `make check` on Ubuntu 15.


Thanks,

Alex Clemmer

Re: Review Request 39262: Windows:[1/3] Moved `os::environ` -> `os::raw::environment`.

[Alex Clemmer]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39262/
---

(Updated Oct. 18, 2015, 6:40 p.m.)


Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
Wu.


Repository: mesos


Description
---

Windows standard headers define a macro, `environ`, that holds the
environment data for the executing process. Unfortunately, the existance
of this macro makes it impossible to use a function called
`os::environ`.

Our solution to this problem in this commit is to move the function
family at `os::environ`* to `os::raw::environment`. This family exists
in the `os::raw` because they are the "unstructured" variants of
functions with the same names that exist in the `os` namespace. For
example, `os::raw::environment` returns a `char**` which is the
"unstructured" equivalent of `os::environment`, which returns a
`map`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
5141c1369af60afd6cd5c70c6295d575ea960a83 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/raw/environment.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
a042d061159c83e1652e7288b90809981d2818c9 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
2b0966889af73238a08e29f1136d0ce286a0ffda 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
ee2a7a79617612ed448e650cc77608c4962fe5a5 

Diff: https://reviews.apache.org/r/39262/diff/


Testing
---

`make check` on Ubuntu 15, OS X 10.10, and running the `check` project on 
Windows 10.


Thanks,

Alex Clemmer

Review Request 39423: Fixed `ExamplesTest.PersistentVolumeFramework` on OS X El Capitan.

[Michael Park]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39423/
---

Review request for mesos and Jie Yu.


Bugs: MESOS-3604
https://issues.apache.org/jira/browse/MESOS-3604


Repository: mesos


Description
---

The `MESOS_LAUNCHER_DIR` was being set to `/src/.libs` rather than 
`/src`. This leads to us attemping to execute 
`/src/.libs/mesos-executor` which doesn't have the correct 
`DYLD_LIBRARY_PATH` set.


Diffs
-

  src/examples/persistent_volume_framework.cpp 
426d33813f8f17397c6ed29b8a8b33df4137bba6 

Diff: https://reviews.apache.org/r/39423/diff/


Testing
---

```
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from ExamplesTest
[ RUN  ] ExamplesTest.PersistentVolumeFramework
[   OK ] ExamplesTest.PersistentVolumeFramework (3391 ms)
[--] 1 test from ExamplesTest (3391 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (3403 ms total)
[  PASSED  ] 1 test.
```


Thanks,

Michael Park

Re: Review Request 39102: Added documentation for JSON resources.

[Guangya Liu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39102/#review103052
---

Ship it!


Ship It!

- Guangya Liu


On 十月 17, 2015, 11:05 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39102/
> ---
> 
> (Updated 十月 17, 2015, 11:05 p.m.)
> 
> 
> Review request for mesos, Adam B and Neil Conway.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for JSON resources.
> 
> 
> Diffs
> -
> 
>   docs/attributes-resources.md f712d094f14426515dabde45f98d6c1ae36c3447 
>   docs/configuration.md 2ab476a2d2c43e309b570d73ecac80e27b296e7e 
> 
> Diff: https://reviews.apache.org/r/39102/diff/
> 
> 
> Testing
> ---
> 
> Viewed the relevant documentation sections ('Attributes and Resources' & 
> 'Configuration') using the mesos-website-container: 
> https://github.com/mesosphere/mesos-website-container
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 39421: Add agent version to /master/slaves endpoint

[Guangya Liu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39421/
---

Review request for mesos and Vinod Kone.


Bugs: MESOS-3735
https://issues.apache.org/jira/browse/MESOS-3735


Repository: mesos


Description
---

Add agent version to /master/slaves endpoint


Diffs
-

  src/master/http.cpp 093f79384916dc08b32b70d3614e0ff314825c42 

Diff: https://reviews.apache.org/r/39421/diff/


Testing
---

root@devstack007:~/src/mesos/bug-fix/mesos/build# curl 
"http://9.111.242.187:5050/master/slaves; 2>/dev/null| jq .
{
  "slaves": [
{
  "version": "0.26.0",  Here
  "used_resources": {
"mem": 0,
"disk": 0,
"cpus": 0
  },
  "unreserved_resources": {
"ports": "[31000-32000]",
"mem": 2808,
"disk": 1399183,
"cpus": 4
  },
  "resources": {
"ports": "[31000-32000]",
"mem": 2808,
"disk": 1399183,
"cpus": 4
  },
  "reserved_resources": {},
  "active": true,
  "attributes": {},
  "hostname": "devstack007.cn.ibm.com",
  "id": "3e0df733-08b3-4883-b3fa-92bdc0c05b2f-S0",
  "offered_resources": {
"mem": 0,
"disk": 0,
"cpus": 0
  },
  "pid": "slave(1)@9.111.242.187:5051",
  "registered_time": 1445174743.70792,
  "reregistered_time": 1445174743.7085
}
  ]
}


Thanks,

Guangya Liu

Re: Review Request 39420: Added '--parent' option and made apply-review.sh call apply-reviews.py.

[Guangya Liu]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39420/#review103056
---


Why not make this as a default option? I think that we need to apply the whole 
chain of patch for development, thoughts? Thanks.


support/apply-reviews.py (lines 271 - 273)


Tab or blank space? If Tab, we should switch to blank space


- Guangya Liu


On 十月 18, 2015, 2:43 a.m., Artem Harutyunyan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39420/
> ---
> 
> (Updated 十月 18, 2015, 2:43 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere, Joseph Wu, Marco Massenzio, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-3468
> https://issues.apache.org/jira/browse/MESOS-3468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added '--parent' option and made apply-review.sh call apply-reviews.py.
> 
> 
> Diffs
> -
> 
>   support/apply-review.sh 6391451542e9e8847ec38e2ad9d9acf552afead3 
>   support/apply-reviews.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39420/diff/
> 
> 
> Testing
> ---
> 
> Tested with python 2.7.
> 
> - with and without '-p'.
> - Tested reviews with and without parents.
> 
> 
> Thanks,
> 
> Artem Harutyunyan
> 
>

Re: Review Request 39421: Add agent version to /master/slaves endpoint

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39421/#review103057
---


Patch looks great!

Reviews applied: [39421]

All tests passed.

- Mesos ReviewBot


On Oct. 18, 2015, 1:29 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39421/
> ---
> 
> (Updated Oct. 18, 2015, 1:29 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3735
> https://issues.apache.org/jira/browse/MESOS-3735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add agent version to /master/slaves endpoint
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 093f79384916dc08b32b70d3614e0ff314825c42 
> 
> Diff: https://reviews.apache.org/r/39421/diff/
> 
> 
> Testing
> ---
> 
> root@devstack007:~/src/mesos/bug-fix/mesos/build# curl 
> "http://9.111.242.187:5050/master/slaves; 2>/dev/null| jq .
> {
>   "slaves": [
> {
>   "version": "0.26.0",  Here
>   "used_resources": {
> "mem": 0,
> "disk": 0,
> "cpus": 0
>   },
>   "unreserved_resources": {
> "ports": "[31000-32000]",
> "mem": 2808,
> "disk": 1399183,
> "cpus": 4
>   },
>   "resources": {
> "ports": "[31000-32000]",
> "mem": 2808,
> "disk": 1399183,
> "cpus": 4
>   },
>   "reserved_resources": {},
>   "active": true,
>   "attributes": {},
>   "hostname": "devstack007.cn.ibm.com",
>   "id": "3e0df733-08b3-4883-b3fa-92bdc0c05b2f-S0",
>   "offered_resources": {
> "mem": 0,
> "disk": 0,
> "cpus": 0
>   },
>   "pid": "slave(1)@9.111.242.187:5051",
>   "registered_time": 1445174743.70792,
>   "reregistered_time": 1445174743.7085
> }
>   ]
> }
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 37336: Simplified the caller interface to process::Subprocess

[Marco Massenzio]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37336/
---

(Updated Oct. 18, 2015, 3:22 p.m.)


Review request for mesos, Joris Van Remoortere and Michael Park.


Changes
---

Fixed compile error which was not caught by local `make && make check`


Bugs: MESOS-3035
https://issues.apache.org/jira/browse/MESOS-3035


Repository: mesos


Description
---

Jira: MESOS-3035

The original API for `process::Subprocess` still left a lot of legwork
to do for the caller; we have now added an `execute()` method
that returns a `Future` (also introduced with this patch) 
which
contains useful information about the command invocation; the exit code;
stdout and, optionally, stderr too.

Once the Future completes, if successful, the caller will be able to 
retrieve
stdout/stderr; whether the command was successful; and whether it received 
a signal


Diffs (updated)
-

  3rdparty/libprocess/include/process/subprocess.hpp 
d2341a53aac7c779668ee80983f73158fd44bff5 
  3rdparty/libprocess/src/subprocess.cpp 
a457cbe35ad33531c49f74550cd570cf28758f5d 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
ac600a551fb1a7782ff33cce204b7819497ef54a 

Diff: https://reviews.apache.org/r/37336/diff/


Testing
---

make check


Thanks,

Marco Massenzio

Re: Review Request 37336: Simplified the caller interface to process::Subprocess

[Mesos ReviewBot]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/37336/#review103060
---


Patch looks great!

Reviews applied: [37336]

All tests passed.

- Mesos ReviewBot


On Oct. 18, 2015, 3:22 p.m., Marco Massenzio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37336/
> ---
> 
> (Updated Oct. 18, 2015, 3:22 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Michael Park.
> 
> 
> Bugs: MESOS-3035
> https://issues.apache.org/jira/browse/MESOS-3035
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Jira: MESOS-3035
> 
> The original API for `process::Subprocess` still left a lot of legwork
> to do for the caller; we have now added an `execute()` method
> that returns a `Future` (also introduced with this patch) 
> which
> contains useful information about the command invocation; the exit code;
> stdout and, optionally, stderr too.
> 
> Once the Future completes, if successful, the caller will be able to 
> retrieve
> stdout/stderr; whether the command was successful; and whether it 
> received a signal
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/subprocess.hpp 
> d2341a53aac7c779668ee80983f73158fd44bff5 
>   3rdparty/libprocess/src/subprocess.cpp 
> a457cbe35ad33531c49f74550cd570cf28758f5d 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> ac600a551fb1a7782ff33cce204b7819497ef54a 
> 
> Diff: https://reviews.apache.org/r/37336/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Marco Massenzio
> 
>

Re: Review Request 39213: Windows: Moved `bootId` to is own file, `stout/os/windows/bootid.hpp`.

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39213/#review103058
---

Ship it!


We can use snake case in stout.


3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/bootid.hpp (line 16)


What about `string`, `sys/time.h`, `stout/error.hpp`, 
`stout/stringify.hpp`,  `stout/strings.hpp`, and `stout/try.hpp`?



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/bootid.hpp (line 17)


What do you need from `os.hpp`? This is a circular dependency..



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/bootid.hpp (lines 
17 - 19)


`string`, `stout/stringify.hpp`



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/bootid.hpp (line 32)


we don't use local namespace introductions like this.


- Joris Van Remoortere


On Oct. 13, 2015, 6:34 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39213/
> ---
> 
> (Updated Oct. 13, 2015, 6:34 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3632
> https://issues.apache.org/jira/browse/MESOS-3632
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Moved `bootId` to is own file, `stout/os/windows/bootid.hpp`.
> 
> (NOTE: this does not close MESOS-3632, but does contribute to its resolution.)
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 5141c1369af60afd6cd5c70c6295d575ea960a83 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/bootid.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/bootid.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/bootid.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39213/diff/
> 
> 
> Testing
> ---
> 
> Built CMake solution and ran `make check` (or equivalent) on Windows 10, 
> Ubuntu 15, OS X 10.10. Built autotools solution and ran `make check` on 
> Ubuntu 15.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39219: Windows: Added support for `slave/state.cpp`.

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39219/#review103062
---



src/slave/state.hpp (line 131)


How is this compiling for you?



src/slave/state.cpp (line 650)


`isError()` reads better.



src/slave/state.cpp (lines 652 - 653)


We now have an error to return from the `Try`



src/slave/state.cpp (lines 730 - 733)


Same as above.


- Joris Van Remoortere


On Oct. 15, 2015, 8:31 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39219/
> ---
> 
> (Updated Oct. 15, 2015, 8:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3615, MESOS-3627, MESOS-3628, MESOS-3629, MESOS-3630, MESOS-3631, 
> MESOS-3633, MESOS-3658, MESOS-3659, MESOS-3660, and MESOS-3693
> https://issues.apache.org/jira/browse/MESOS-3615
> https://issues.apache.org/jira/browse/MESOS-3627
> https://issues.apache.org/jira/browse/MESOS-3628
> https://issues.apache.org/jira/browse/MESOS-3629
> https://issues.apache.org/jira/browse/MESOS-3630
> https://issues.apache.org/jira/browse/MESOS-3631
> https://issues.apache.org/jira/browse/MESOS-3633
> https://issues.apache.org/jira/browse/MESOS-3658
> https://issues.apache.org/jira/browse/MESOS-3659
> https://issues.apache.org/jira/browse/MESOS-3660
> https://issues.apache.org/jira/browse/MESOS-3693
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `slave/state.cpp`.
> 
> 
> Diffs
> -
> 
>   src/slave/state.hpp 5a1a9bb2c86639612a8f065b7a66c8179696297a 
>   src/slave/state.cpp 81c4b96d879f974f0dfba3fb977184122eab 
> 
> Diff: https://reviews.apache.org/r/39219/diff/
> 
> 
> Testing
> ---
> 
> Built CMake solution and ran `make check` (or equivalent) on Windows 10, 
> Ubuntu 15, OS X 10.10. Built autotools solution and ran `make check` on 
> Ubuntu 15.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39018: Added JSON parsing for Resources.

[Michael Park]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39018/#review103063
---



src/common/resources.cpp (lines 482 - 530)


How about a structure like the following?

```cpp
Resources result;

// Populate `result` based on which format.
Try resourcesJSON = JSON::parse(text);
if (resourcesJSON.isSome()) {
  result = internal::convertJSON(resourcesJSON.get(), defaultRole);
} else {
  foreach (...) {
...
result += resource.get();
  }
}

// Validate the result regardless of what format
Option validate = validateCommandLineResources(result);
if (validate.isSome()) {
  return validate.get();
}

return result;
```

`validateCommandLineResources` is as suggested before, with the 
`conflictingTypes` logic encapsulated within it.

I noticed we only perform the minimal validation necessary for the 
semicolon-delimited string format currently.
That is, we only do the `conflictingTypes` check. While this is sufficient 
in the current state of the format,
I think it simplifies the code to just do the full check. It's also 
future-proof if we need to extend the string format later on.


- Michael Park


On Oct. 14, 2015, 3:50 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 14, 2015, 3:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-2467
> https://issues.apache.org/jira/browse/MESOS-2467
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes code changes necessary for JSON parsing of Resources. 
> Documentation changes will be posted soon in another review 
> (https://reviews.apache.org/r/39102/).
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 6c3a065945eb56dc88df9c977e5ca11d4cbcbf61 
>   include/mesos/v1/resources.hpp fe8925ac851b74d1b37919f00afc7ed816f47ea5 
>   src/common/resources.cpp 601388c35a1bff37c58e753d1870d53b8d0af2d1 
>   src/tests/resources_tests.cpp 6584fc6c39e6ffe9f8085576677dcc669f127697 
>   src/v1/resources.cpp dc868903472f8f3a1ddc56092e3f8f81d953ce39 
> 
> Diff: https://reviews.apache.org/r/39018/diff/
> 
> 
> Testing
> ---
> 
> New code was added to `ResourcesTest.ParsingFromJSON`, and two new tests were 
> added: `ResourcesTest.ParsingFromJSONWithRoles` and 
> `ResourcesTest.ParsingFromJSONError`. These attempt to test common error 
> scenarios that might show up in user-supplied JSON.
> 
> `make check` was used to confirm that all tests pass, with one notable 
> failure (ResourcesTest.ParsingFromJSONError) related to a picojson issue 
> tracked here: https://issues.apache.org/jira/browse/MESOS-3698
> 
> The original resources parsing format is used throughout many other tests 
> (check `src/tests/sorter_tests.cpp`, for example), so it's clear that the 
> original parsing continues to work correctly.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39217: Windows: Added `stout/os/ftruncate.hpp`.

[Joris Van Remoortere]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39217/#review103059
---

Ship it!


I think there are places like `os/posix/fork.hpp` that we need to adjust to use 
this ftruncate.


3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ftruncate.hpp (line 
16)


`sys/types.h`
`stout/error.hpp`
`stout/stringify.hpp`



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ftruncate.hpp (lines 
27 - 28)


`stringify()`?



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
(lines 19 - 20)


order
`stout/error.hpp`
`stout/stringify.hpp`



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
(lines 30 - 31)


`stringify()`?


- Joris Van Remoortere


On Oct. 16, 2015, 8 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39217/
> ---
> 
> (Updated Oct. 16, 2015, 8 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `stout/os/ftruncate.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/ftruncate.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/ftruncate.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39217/diff/
> 
> 
> Testing
> ---
> 
> Built CMake solution and ran `make check` (or equivalent) on Windows 10, 
> Ubuntu 15, OS X 10.10. Built autotools solution and ran `make check` on 
> Ubuntu 15.
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>