Re: Review Request 39300: Fixed flaky ReservationEndpointsTest.AvailableResources test.

[Michael Park]

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

(Updated Oct. 14, 2015, 4:21 a.m.)


Review request for mesos and Ben Mahler.


Changes
---

Moved the `WillRepeatedly(DoDefault())` to the end of the test accompanied with 
a comment.
Updated 2 other such instances in this file to follow the same pattern.


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


Repository: mesos


Description
---

The following failure was encountered on buildbot: 
https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu:14.04,label_exp=docker%7C%7CHadoop/921/console

```
I1013 09:54:08.882694 29149 master.cpp:5559] Processing TEARDOWN call for 
framework 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39- (default) at 
scheduler-62c161d7-60e9-4361-aae1-6431e60035f6@172.17.5.161:57074
I1013 09:54:08.882822 29149 master.cpp:5571] Removing framework 
4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39- (default) at 
scheduler-62c161d7-60e9-4361-aae1-6431e60035f6@172.17.5.161:57074
../../src/tests/reservation_endpoints_tests.cpp:184: Failure
Mock function called more times than expected - taking default action specified 
at:
../../src/tests/mesos.hpp:1518:
Function call: recoverResources(@0x2b1be400cc10 
4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-, @0x2b1be4018f40 
4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-S0, @0x2b1bc22fc390 { cpus(*):2, 
mem(*):1024, disk(*):1024, ports(*):[31000-32000] }, @0x2b1bc22fc3d0 40-byte 
object <01-00 00-00 1C-2B 00-00 0B-00 00-00 00-00 00-00 00-00 00-00 00-00 00-00 
00-00 00-00 00-00 00-00 00-00 80-3F 00-00 00-00>)
 Expected: to be called once
   Actual: called twice - over-saturated and active
I1013 09:54:08.884042 29149 hierarchical.hpp:599] Deactivated framework 
4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-
I1013 09:54:08.884371 29149 hierarchical.hpp:1103] Recovered cpus(*):2; 
mem(*):1024; disk(*):1024; ports(*):[31000-32000] (total: cpus(*):2; 
mem(*):1024; disk(*):1024; ports(*):[31000-32000], allocated: ) on slave 
4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-S0 from framework 
4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-
I1013 09:54:08.884469 29149 hierarchical.hpp:552] Removed framework 
4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-
```

In `ReservationEndpointsTest.AvailableResources`, the test ends with the 
framework holding an offer.
The issue is that there is only a `WillOnce` on `recoverResources`, but it 
should ignore subsequent calls with `WillRepeatedly`
since resources are expected to recover on framework teardown.

It seems that there is a race in the test teardown logic, but this fix is 
applicable regardless.


Diffs (updated)
-

  src/tests/reservation_endpoints_tests.cpp 
d41185241ae8f1f4ad4f9e88b7dea6fdac991ae7 

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


Testing
---


Thanks,

Michael Park

Re: Review Request 39300: Fixed flaky ReservationEndpointsTest.AvailableResources test.

[Michael Park]

> On Oct. 14, 2015, 12:44 a.m., Ben Mahler wrote:
> > src/tests/reservation_endpoints_tests.cpp, line 187
> > 
> >
> > Shall we put this down before stopping the driver to be a bit clearer? 
> > A comment there would be helpful. :)

Great suggestion! I've updated the other 2 instances in this file as well.


- Michael


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


On Oct. 14, 2015, 4:21 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39300/
> ---
> 
> (Updated Oct. 14, 2015, 4:21 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-3411
> https://issues.apache.org/jira/browse/MESOS-3411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The following failure was encountered on buildbot: 
> https://builds.apache.org/job/Mesos/COMPILER=gcc,CONFIGURATION=--verbose,OS=ubuntu:14.04,label_exp=docker%7C%7CHadoop/921/console
> 
> ```
> I1013 09:54:08.882694 29149 master.cpp:5559] Processing TEARDOWN call for 
> framework 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39- (default) at 
> scheduler-62c161d7-60e9-4361-aae1-6431e60035f6@172.17.5.161:57074
> I1013 09:54:08.882822 29149 master.cpp:5571] Removing framework 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39- (default) at 
> scheduler-62c161d7-60e9-4361-aae1-6431e60035f6@172.17.5.161:57074
> ../../src/tests/reservation_endpoints_tests.cpp:184: Failure
> Mock function called more times than expected - taking default action 
> specified at:
> ../../src/tests/mesos.hpp:1518:
> Function call: recoverResources(@0x2b1be400cc10 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-, @0x2b1be4018f40 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-S0, @0x2b1bc22fc390 { cpus(*):2, 
> mem(*):1024, disk(*):1024, ports(*):[31000-32000] }, @0x2b1bc22fc3d0 40-byte 
> object <01-00 00-00 1C-2B 00-00 0B-00 00-00 00-00 00-00 00-00 00-00 00-00 
> 00-00 00-00 00-00 00-00 00-00 00-00 80-3F 00-00 00-00>)
>  Expected: to be called once
>Actual: called twice - over-saturated and active
> I1013 09:54:08.884042 29149 hierarchical.hpp:599] Deactivated framework 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-
> I1013 09:54:08.884371 29149 hierarchical.hpp:1103] Recovered cpus(*):2; 
> mem(*):1024; disk(*):1024; ports(*):[31000-32000] (total: cpus(*):2; 
> mem(*):1024; disk(*):1024; ports(*):[31000-32000], allocated: ) on slave 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-S0 from framework 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-
> I1013 09:54:08.884469 29149 hierarchical.hpp:552] Removed framework 
> 4b0845cd-7ce9-4e7a-b5d1-bcf1c413ca39-
> ```
> 
> In `ReservationEndpointsTest.AvailableResources`, the test ends with the 
> framework holding an offer.
> The issue is that there is only a `WillOnce` on `recoverResources`, but it 
> should ignore subsequent calls with `WillRepeatedly`
> since resources are expected to recover on framework teardown.
> 
> It seems that there is a race in the test teardown logic, but this fix is 
> applicable regardless.
> 
> 
> Diffs
> -
> 
>   src/tests/reservation_endpoints_tests.cpp 
> d41185241ae8f1f4ad4f9e88b7dea6fdac991ae7 
> 
> Diff: https://reviews.apache.org/r/39300/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 39076: CMake: Added ability of Windows builds to include protobuf headers.

[Artem Harutyunyan]

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


LGTM.


3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake (lines 62 - 64)


micro-nit: shall the values be aligned here and below (in the else branch)?


- Artem Harutyunyan


On Oct. 12, 2015, 10:51 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39076/
> ---
> 
> (Updated Oct. 12, 2015, 10:51 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake: Added ability of Windows builds to include protobuf headers.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> ee1c74d31e28136bf289f4100d79a8ce568cd3af 
> 
> Diff: https://reviews.apache.org/r/39076/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 37996: Added InheritanceTree a tree based container where children nodes inherit the values associated with their parent.

[Alexander Rojas]

> On Oct. 6, 2015, 2:39 p.m., Bernd Mathiske wrote:
> > Ship It!
> 
> Ben Mahler wrote:
> I don't think we should introduce this into stout in its current form. I 
> realize you're planning to use this for authentication stuff, but looking at 
> this on its own, it seems like a confusing abstraction. Why would we couple 
> the notion of a Tree with the semantics around properties and property 
> inheritance?
> 
> Till Toenshoff wrote:
> This code is being used in libprocess. So the options are libprocess or 
> stout for introducing it. I believe it would be a better fit for stout than 
> for libprocess as it is a data structure implementation that has no threading 
> or process specifics. Given that it already is in a reusable state, I think 
> that we should go as proposed by this RR.
> 
> Bernd Mathiske wrote:
> @bmahler, what would you rather have us do? This structure is quite close 
> to Boost's PropertyTree 
> (http://www.boost.org/doc/libs/1_59_0/doc/html/property_tree.html), and we 
> would have loved to use that one. But it does not feature inheritance, which 
> seems a natural, essential thing to have in at least some trees.
> 
> Ben Mahler wrote:
> Hm... the "inheritance" stuff that you guys need seems to just be a walk 
> of the tree from the root to the leaf you're interested in, collecting 
> properties of nodes along the way. Any reason you can't layer that 
> functionality on top or, even better, on the side via a free standing 
> function so that you don't need to introduce a new data structure?
> 
> Jie Yu wrote:
> +1 if we can compose/inherit from boost property tree rather than 
> implementing our own.
> 
> Alexander Rojas wrote:
> Property tree as of the moment is not distributed with Mesos. However, 
> there is a reason why it wasn't added, and it is that it will require a 
> double parsing of the path (one when asking the ptree, the other manually to 
> verify every subpath).
> 
> Ben Mahler wrote:
> In other words, you were concerned about performance?

That and that pulling property tree from boost will include the whole boost 
spirit. Still, I will withdrawn this patch and follow your suggestion of puting 
this structure only where it will be used.


- Alexander


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


On Oct. 7, 2015, 11:54 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37996/
> ---
> 
> (Updated Oct. 7, 2015, 11:54 a.m.)
> 
> 
> Review request for mesos, Adam B, Benjamin Hindman, Bernd Mathiske, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3231
> https://issues.apache.org/jira/browse/MESOS-3231
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduces the `InheritanceTree` class which allows to create a tree where 
> nodes can be tag with some property. This property is then inherited by 
> children nodes.
> 
> Two behaviors are implemented, overriding, i.e. Adding a property to the 
> child node of another node with a property already will result in the 
> ancestor property being lost. The second behavior, accumulating, takes a 
> function and accumulates
> properties of all ancestors.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/Makefile.am 
> 76e1674e08bbe65a4fdf86731823a61f231d6d12 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/inheritancetree.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
>   3rdparty/libprocess/3rdparty/stout/tests/inheritancetree_tests.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/37996/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

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/
---

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 
5d2f39d9a9d963225bf463572cb8fe99dd9aa6f5 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
2b0966889af73238a08e29f1136d0ce286a0ffda 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
e6d36ec1bf414b52d0899f0edf83e0ad8910dd0e 

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 39263: Windows:[2/3] transitioned libprocess to use `os::raw::` family.

[Alex Clemmer]

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

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

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

[Alex Clemmer]

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

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 39258: Add filesystem isolator with command executor test.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38900, 39258]

All tests passed.

- Mesos ReviewBot


On Oct. 13, 2015, 3:40 a.m., Timothy Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39258/
> ---
> 
> (Updated Oct. 13, 2015, 3:40 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add filesystem isolator with command executor test.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/launcher/executor.cpp 50b3c6e319f4b1e08c8ebcdd9f161e19bb14d390 
>   src/slave/containerizer/provisioner/provisioner.hpp 
> 912fc5abadb1219fc4baec1a751010522bc7a7d0 
>   src/slave/containerizer/provisioner/provisioner.cpp 
> 30a85a52ee5986c6e1652edfd08ae881280b23ab 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> 1e332e0d36ac2812456514aa030f995b3a07dca1 
>   src/tests/containerizer/store.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/39258/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Timothy Chen
> 
>

Re: Review Request 39099: Changed secret field in Credential from 'bytes' to 'string' for V1

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 十月 13, 2015, 8 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39099/
> ---
> 
> (Updated 十月 13, 2015, 8 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change is introduced in dependant patch and we should update V1 proto as 
> well.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto eadbc9d 
> 
> Diff: https://reviews.apache.org/r/39099/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

[Isabel Jimenez]

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

(Updated Oct. 13, 2015, 7:53 a.m.)


Review request for mesos and Michael Park.


Changes
---

+ test


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


Repository: mesos


Description
---

When decoding the JSON credential file into the Credential protobuf object, the 
'secret' which is in 'bytes' is mapped into base64 string automatically by 
protobuf from JSON. This creates an unintended behavior, forcing users to 
encode in base64 their secret when wanting to pass a JSON file to the 
--credentials flag.


Diffs (updated)
-

  include/mesos/mesos.proto 4a16be1 
  src/examples/java/TestExceptionFramework.java 78720b0 
  src/examples/java/TestFramework.java aad94c0 
  src/tests/credentials_tests.cpp ced27c4 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Review Request 39259: Enable scheduler driver can use Call::REQUEST to request resource

[Guangya Liu]

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

Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Enable scheduler driver can use Call::REQUEST to request resource


Diffs
-

  src/sched/sched.cpp 724d7c0cef073342436de7cf5fe834fffbf0 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 39092: CMake:[2/3] prepared process library tests to run with `make check`.

[Artem Harutyunyan]

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


LGTM.

- Artem Harutyunyan


On Oct. 7, 2015, 9 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39092/
> ---
> 
> (Updated Oct. 7, 2015, 9 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3597
> https://issues.apache.org/jira/browse/MESOS-3597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Right now running the Mesos tests from the default tests target will (1)
> aggregate all our tests into a semi-useless report that hides all the
> errors, and (2) strip all our colors when you do pass the verbosity flag
> to get rid of the "report" structure.
> 
> This commit will prepare us to run the tests with with color, without
> this default reporting structure. We do this by moving the tests target
> to a `CACHE STRING` which can be referenced by the tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> ea6db09e1a1aa01450aee93814e07f09feae7ac9 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
> 
> Diff: https://reviews.apache.org/r/39092/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39091: CMake:[1/3] Prepared stout tests to run with `make check`.

[Artem Harutyunyan]

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


LGTM.

- Artem Harutyunyan


On Oct. 7, 2015, 9 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39091/
> ---
> 
> (Updated Oct. 7, 2015, 9 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3597
> https://issues.apache.org/jira/browse/MESOS-3597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Right now running the Mesos tests from the default tests target will
> (1) aggregate all our tests into a semi-useless report that hides all
> the errors, and (2) strip all our colors when you do pass the verbosity
> flag to get rid of the "report" structure.
> 
> This commit will prepare us to run the tests with with color, without
> this default reporting structure. We do this by moving the tests target
> to a `CACHE STRING` which can be referenced by the tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
> c4d1b7b3f896f8037d922fafcdbdd0e31a70e140 
>   3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
> 94292f8a46ec31bbaf6e52f48109322bbe123f70 
> 
> Diff: https://reviews.apache.org/r/39091/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

[Guangya Liu]

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



src/tests/credentials_tests.cpp (line 116)


Can you pls add some comments here for this unit test?


- Guangya Liu


On 十月 13, 2015, 7:53 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> ---
> 
> (Updated 十月 13, 2015, 7:53 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3560
> https://issues.apache.org/jira/browse/MESOS-3560
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When decoding the JSON credential file into the Credential protobuf object, 
> the 'secret' which is in 'bytes' is mapped into base64 string automatically 
> by protobuf from JSON. This creates an unintended behavior, forcing users to 
> encode in base64 their secret when wanting to pass a JSON file to the 
> --credentials flag.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4a16be1 
>   src/examples/java/TestExceptionFramework.java 78720b0 
>   src/examples/java/TestFramework.java aad94c0 
>   src/tests/credentials_tests.cpp ced27c4 
> 
> Diff: https://reviews.apache.org/r/39098/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

[Isabel Jimenez]

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

(Updated Oct. 13, 2015, 8:45 a.m.)


Review request for mesos and Michael Park.


Changes
---

review comments


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


Repository: mesos


Description
---

When decoding the JSON credential file into the Credential protobuf object, the 
'secret' which is in 'bytes' is mapped into base64 string automatically by 
protobuf from JSON. This creates an unintended behavior, forcing users to 
encode in base64 their secret when wanting to pass a JSON file to the 
--credentials flag.


Diffs (updated)
-

  include/mesos/mesos.proto 4a16be1 
  src/examples/java/TestExceptionFramework.java 78720b0 
  src/examples/java/TestFramework.java aad94c0 
  src/tests/credentials_tests.cpp ced27c4 

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


Testing
---

make check


Thanks,

Isabel Jimenez

Re: Review Request 39259: Enable scheduler driver can use Call::REQUEST to request resource

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39259]

All tests passed.

- Mesos ReviewBot


On Oct. 13, 2015, 7:54 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39259/
> ---
> 
> (Updated Oct. 13, 2015, 7:54 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-3715
> https://issues.apache.org/jira/browse/MESOS-3715
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable scheduler driver can use Call::REQUEST to request resource
> 
> 
> Diffs
> -
> 
>   src/sched/sched.cpp 724d7c0cef073342436de7cf5fe834fffbf0 
> 
> Diff: https://reviews.apache.org/r/39259/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

[Michael Park]

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

Ship it!


I added the comments to the test you added, but some of them apply to 
`AuthenticatedSlaveText` as well. Let's fix those as well :)


src/tests/credentials_tests.cpp (line 119)


Let's rename this `s/flags/masterFlags/` and move it to just above 
`flags.load(values, true)` below.



src/tests/credentials_tests.cpp (line 121)


`path::join` returns a temporary string right?
In which case: `s/const string &/string/`.



src/tests/credentials_tests.cpp (line 145)


Add newline after



src/tests/credentials_tests.cpp (line 147)


Let's just use `s/flags.credentials.get().value/path/` here?



src/tests/credentials_tests.cpp (line 158)


Remove newline.


- Michael Park


On Oct. 13, 2015, 7:53 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> ---
> 
> (Updated Oct. 13, 2015, 7:53 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3560
> https://issues.apache.org/jira/browse/MESOS-3560
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When decoding the JSON credential file into the Credential protobuf object, 
> the 'secret' which is in 'bytes' is mapped into base64 string automatically 
> by protobuf from JSON. This creates an unintended behavior, forcing users to 
> encode in base64 their secret when wanting to pass a JSON file to the 
> --credentials flag.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4a16be1 
>   src/examples/java/TestExceptionFramework.java 78720b0 
>   src/examples/java/TestFramework.java aad94c0 
>   src/tests/credentials_tests.cpp ced27c4 
> 
> Diff: https://reviews.apache.org/r/39098/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 39098: Changed secret field in Credential from 'bytes' to 'string'

[Michael Park]

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

Ship it!


I've committed this with the following minor style fixes!


src/tests/credentials_tests.cpp (line 55)


`s/> >/>>/`



src/tests/credentials_tests.cpp (line 61)


`s/> >/>>/`



src/tests/credentials_tests.cpp (line 85)


`s/const std::string&/std::string/`



src/tests/credentials_tests.cpp (line 86)


newline after this line



src/tests/credentials_tests.cpp (line 88)


3 space indent => 4 space indent



src/tests/credentials_tests.cpp (line 142)


3 space indent => 4 space indent



src/tests/credentials_tests.cpp (lines 146 - 147)


Fits in one line.


- Michael Park


On Oct. 13, 2015, 8:45 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39098/
> ---
> 
> (Updated Oct. 13, 2015, 8:45 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3560
> https://issues.apache.org/jira/browse/MESOS-3560
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When decoding the JSON credential file into the Credential protobuf object, 
> the 'secret' which is in 'bytes' is mapped into base64 string automatically 
> by protobuf from JSON. This creates an unintended behavior, forcing users to 
> encode in base64 their secret when wanting to pass a JSON file to the 
> --credentials flag.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 4a16be1 
>   src/examples/java/TestExceptionFramework.java 78720b0 
>   src/examples/java/TestFramework.java aad94c0 
>   src/tests/credentials_tests.cpp ced27c4 
> 
> Diff: https://reviews.apache.org/r/39098/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

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

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [39076, 39091, 39092, 39093, 39096, 39097, 39180, 39181, 
39201, 39182, 39202, 39203, 39204, 39207, 39208, 39209, 39210, 39213, 39217, 
39218, 39219, 39220]

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

Error:
 2015-10-13 11:01:08 URL:https://reviews.apache.org/r/39220/diff/raw/ 
[4689/4689] -> "39220.patch" [1]
error: patch failed: CMakeLists.txt:89
error: CMakeLists.txt: patch does not apply
error: patch failed: src/CMakeLists.txt:223
error: src/CMakeLists.txt: patch does not apply
Failed to apply patch

- Mesos ReviewBot


On Oct. 13, 2015, 9:10 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39265/
> ---
> 
> (Updated Oct. 13, 2015, 9:10 a.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 38869: Added static->dynamic transformation to Allocator.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [38869]

All tests passed.

- Mesos ReviewBot


On Oct. 13, 2015, 8:51 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Oct. 13, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, Joseph 
> Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/master/allocator/mesos/hierarchical.hpp 
> d57c55e2f173c3054a04322a33ba590c67b82e4e 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 十月 13, 2015, 8:51 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated 十月 13, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, Joseph 
> Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/master/allocator/mesos/hierarchical.hpp 
> d57c55e2f173c3054a04322a33ba590c67b82e4e 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 39259: Enable scheduler driver can use Call::REQUEST to request resource

[Guangya Liu]

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

(Updated 十月 13, 2015, 1:29 p.m.)


Review request for mesos, Ben Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Enable scheduler driver can use Call::REQUEST to request resource


Diffs
-

  src/sched/sched.cpp 724d7c0cef073342436de7cf5fe834fffbf0 

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


Testing (updated)
---

Ubuntu 14.04 
make
make check


Thanks,

Guangya Liu

Re: Review Request 39230: Added check for SASL deprecation into configuration phase.

[Till Toenshoff]

> On Oct. 12, 2015, 7:37 p.m., James Peach wrote:
> > It's marginal, but I think ```AC_LANG_PROGRAM``` is a little clearer that 
> > ```AC_LANG_SOURCE```.
> > 
> > If you are going to rely on the diagnostic pragmas, an alternative is to 
> > simply use those to disable the warning in the SASL wrappers which is a 
> > little more tightly scoped.

After some discussions I feel that wrapping and including the pragmas may 
actually be a better solution indeed.


- Till


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


On Oct. 12, 2015, 1:25 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39230/
> ---
> 
> (Updated Oct. 12, 2015, 1:25 p.m.)
> 
> 
> Review request for mesos and switched to 'mcypark'.
> 
> 
> Bugs: MESOS-3030
> https://issues.apache.org/jira/browse/MESOS-3030
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch also removes some useless CFLAGS handling on the SASL CRAM-MD5 
> checks of the configuration.
> 
> 
> Diffs
> -
> 
>   configure.ac 66f9b32773861d2921d99189e0fbcaea48c456a9 
> 
> Diff: https://reviews.apache.org/r/39230/diff/
> 
> 
> Testing
> ---
> 
> ./configure && make check 
> OSX, clang3.7 + gcc4.9
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 37813: Added additional diagnostic output when a fetcher cache test fails.

[Till Toenshoff]

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

Ship it!



src/tests/fetcher_cache_tests.cpp (line 567)


To prevent further confusion, wouldn't it make sense to call this 
`sandboxPath` here and everywhere else instead - like you did before?


- Till Toenshoff


On Oct. 12, 2015, 4:25 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37813/
> ---
> 
> (Updated Oct. 12, 2015, 4:25 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Joseph Wu, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3235
> https://issues.apache.org/jira/browse/MESOS-3235
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Dumps all involved task/executor sandbox contents in test tear down
> only if a failure occurred.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_cache_tests.cpp 7e194dc6e2b2d8b857e61d1a18d696545a86ce9f 
> 
> Diff: https://reviews.apache.org/r/37813/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX, where the bug showed up.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

[Joris Van Remoortere]

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



src/master/http.cpp (line 787)


Let's add the corresponding HELP function for this.



src/master/http.cpp (lines 790 - 800)


I don't think we need the `else if`s here.

```
if (a) {
  return ...;
}

if (b) {
  return ...;
}
```
This provides the opportunity to comment above each block and makes it 
easier to see each termination block.



src/master/http.cpp (line 799)


Can we mentioned what the method is in this error? You can use the 
maintenance endpoints as an example.



src/master/master.hpp (line 842)


s/actor is/actor. It is/



src/master/master.hpp (line 1020)


Let's add a comment that we are starting to partition some of the Admin 
APIs into classes to keep related endpoints together in a "module".


- Joris Van Remoortere


On Oct. 13, 2015, 8:55 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36913/
> ---
> 
> (Updated Oct. 13, 2015, 8:55 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Alex Clemmer, 
> and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added /quota HTTP Endpoint for Quota handling.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/master/http.cpp 4b9f9ed005a4af2897171659d15168955cc60660 
>   src/master/master.hpp 9d957519bb0f717526af9b2717dc870fae93c20f 
>   src/master/master.cpp 6bee4f351c3fd0fb72f64bbc863968e4786b318b 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36913/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 36913: Added /quota HTTP Endpoint for Quota handling.

[Joerg Schad]

> On Oct. 13, 2015, 11:54 a.m., Joris Van Remoortere wrote:
> > src/master/http.cpp, line 787
> > 
> >
> > Let's add the corresponding HELP function for this.

See Alex's earlier comment about providing the Help function once the semantics 
are finalized.


- Joerg


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


On Oct. 13, 2015, 8:55 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36913/
> ---
> 
> (Updated Oct. 13, 2015, 8:55 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Bernd Mathiske, Alex Clemmer, 
> and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3073
> https://issues.apache.org/jira/browse/MESOS-3073
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added /quota HTTP Endpoint for Quota handling.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 828dbb690841c561816811dfbb044aa3afead89d 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/master/http.cpp 4b9f9ed005a4af2897171659d15168955cc60660 
>   src/master/master.hpp 9d957519bb0f717526af9b2717dc870fae93c20f 
>   src/master/master.cpp 6bee4f351c3fd0fb72f64bbc863968e4786b318b 
>   src/master/quota_handler.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/36913/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

[Alexander Rojas]

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

Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.


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


Repository: mesos


Description
---

When using the same socket to send multiple HTTP requests to different actors. 
If the actor responsible for handling the first request is stuck handling 
another event while a subsequent request can reply immediatly, the order of the 
responses is altered violating HTTP Pipelining.

This patch fixes that problem enforcing that every response is sent in the 
order the corresponding request arrived. It also adds a test to reproduce the 
issue and verify the fix works.


Diffs
-

  3rdparty/libprocess/include/process/event.hpp 
16ddbd77afa6efdf6bad201aa497ee102aa863ae 
  3rdparty/libprocess/src/process.cpp d1c81f1d244f02bf42cab97198587ce1b8c7c407 
  3rdparty/libprocess/src/tests/http_tests.cpp 
38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 

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


Testing
---

make check


Thanks,

Alexander Rojas

Re: Review Request 39093: CMake:[3/3] Add `make check` target.

[Artem Harutyunyan]

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


LGTM.

- Artem Harutyunyan


On Oct. 12, 2015, 12:41 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39093/
> ---
> 
> (Updated Oct. 12, 2015, 12:41 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3597
> https://issues.apache.org/jira/browse/MESOS-3597
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> CMake:[3/3] Add `make check` target.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt a5a66c13496a95bb7095504a4645909d887fb49f 
> 
> Diff: https://reviews.apache.org/r/39093/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38747: Adding digest utilities

[Jojy Varghese]

> On Oct. 13, 2015, 12:54 a.m., Jiang Yan Xu wrote:
> > 3rdparty/libprocess/include/process/digest.hpp, lines 85-120
> > 
> >
> > These templates (here and below) have different levels of 
> > specializations, traits, typedefs in various places, which is hard to 
> > understand.
> > 
> > Do you think the following is simpler?
> > 
> > For each digest type, there is one **low-level** implementation, i.e., 
> > Init, Update and Final are called separatedly but they expose one common 
> > interface (without digest-type specific) arguments.
> > 
> > ```
> > class DigestImpl
> > {
> > public:
> >   void init() == 0; 
> >   int update(const void*, size_t) == 0; 
> >   int final(uint8_t*) == 0;
> >   static Try create(...);
> > };
> > ```
> > 
> > You have a low implementation for each type that inherits this 
> > interface and encapsulates its specific context variables in its member 
> > variables. The low-level implementation should be simply calling openssl 
> > APIs so it should be short and has no more redundant code among different 
> > implementations than specializations in different places and this 
> > consolidates handling of specific digest-types in one single place.
> > 
> > The high-level DigestUtil implementation or simply, the static generic 
> > method implementation can just create a low-level impl class and you can 
> > put common logic there.
> > 
> > What do you think?

I should probably try to reason about the choice of templates (as opposed to 
runtime polymorphism)

SSL digest API can be seen as "logic" and "structure". The logic is the same 
for all SHAs. Only structure is different. The difference in structure can be 
represented as template traits. This allows the difference between SHAs to be 
viewed as *declarative* (as opposed to *procedural*).

This has following advantages:
1. Any new SHA digest can be added by just adding the type traits 
*declaratively*. For example, to add SHA224, we just need to add the following 
lines:

```
template <> 
 
struct DigestTypeTraits 
 
{   
 
  typedef SHA224_CTX ctx_type;  
 
  static const size_t digest_length = SSHA224_DIGEST_LENGTH;
  
};

template <> 
 
DigestFunctionTraits::Init_fn   
 
  DigestFunctionTraits::Init = SHA224_Init; 
 

 

 
template <> 
 
DigestFunctionTraits::Update_fn 
 
  DigestFunctionTraits::Update = SHA224_Update; 
 

 

 
template <> 
 
DigestFunctionTraits::Final_fn  
 
  DigestFunctionTraits::Final = SHA224_Final; 
  
```

If you notice all of the above new code is declarative and does not add any new 
logic. This is possible because we have abstracted out the difference between 
SHA implementations into templates.

2. At the call site, clients can call digests simply by:

```
 DigestUtil::digest(string)

```

This avoids  runtime polymorphism.


> On Oct. 13, 2015, 12:54 a.m., Jiang Yan Xu wrote:
> > 3rdparty/libprocess/include/process/digest.hpp, lines 159-160
> > 
> >
> > About templatization. I previously commented that the implementation 
> > should be put in the CPP file.
> > 
> > You use templates but your interafce is not templatized and used only 
> > by this single component. Can't all the templates just be in the same 
> > source file as the caller so that the header only has the API declarations?
> > 
> > This of course becomes a moot point is we don't use template as I was 
> > suggesting above.

One of the reasons the templates are in header file is to allow client code 
like:

```
DigestUtil::digest(string)
```


- Jojy


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


On Oct. 12, 2015, 9:14 p.m., Jojy Varghese wrote:
> 
> 

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

[Alex Clemmer]

> On Oct. 13, 2015, 11:01 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [39076, 39091, 39092, 39093, 39096, 39097, 39180, 39181, 
> > 39201, 39182, 39202, 39203, 39204, 39207, 39208, 39209, 39210, 39213, 
> > 39217, 39218, 39219, 39220]
> > 
> > Failed command: ./support/apply-review.sh -n -r 39220
> > 
> > Error:
> >  2015-10-13 11:01:08 URL:https://reviews.apache.org/r/39220/diff/raw/ 
> > [4689/4689] -> "39220.patch" [1]
> > error: patch failed: CMakeLists.txt:89
> > error: CMakeLists.txt: patch does not apply
> > error: patch failed: src/CMakeLists.txt:223
> > error: src/CMakeLists.txt: patch does not apply
> > Failed to apply patch

39220 is a review that used to apply and no longer does. Looks like it's time 
to update the whole chain!


- Alex


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


On Oct. 13, 2015, 9:10 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39265/
> ---
> 
> (Updated Oct. 13, 2015, 9:10 a.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 39276: Fixed a bug in which under certains circumstances HTTP 1.1 Pipelining is not respected.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39276]

All tests passed.

- Mesos ReviewBot


On Oct. 13, 2015, 3 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39276/
> ---
> 
> (Updated Oct. 13, 2015, 3 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Bernd Mathiske, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3705
> https://issues.apache.org/jira/browse/MESOS-3705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When using the same socket to send multiple HTTP requests to different 
> actors. If the actor responsible for handling the first request is stuck 
> handling another event while a subsequent request can reply immediatly, the 
> order of the responses is altered violating HTTP Pipelining.
> 
> This patch fixes that problem enforcing that every response is sent in the 
> order the corresponding request arrived. It also adds a test to reproduce the 
> issue and verify the fix works.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/event.hpp 
> 16ddbd77afa6efdf6bad201aa497ee102aa863ae 
>   3rdparty/libprocess/src/process.cpp 
> d1c81f1d244f02bf42cab97198587ce1b8c7c407 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 38f3ad7e46f5b6ef4850cdf7fdcc115715e98472 
> 
> Diff: https://reviews.apache.org/r/39276/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 38627: Adds an overload of ModuleManager::create() allowing overriding parameters programatically

[Niklas Nielsen]

> On Oct. 12, 2015, 5:54 a.m., Marco Massenzio wrote:
> > Would it be possible to add a few unit tests, also to show usage patterns? 
> > especially given the absence of any documentation, it's kinda difficult to 
> > figure out how is this "intended to work" and, without tests, whether it 
> > works at all.

Alex: Ping ^^


- Niklas


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


On Oct. 2, 2015, 1:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38627/
> ---
> 
> (Updated Oct. 2, 2015, 1:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Bernd Mathiske, Niklas Nielsen, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3232
> https://issues.apache.org/jira/browse/MESOS-3232
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allows developers to provide their own parameters when loading modules 
> instead of using the ones provided by the user when loading Mesos. This helps 
> to deal with default modules (those used when the user doesn't provide any), 
> and for testing of the modules.
> 
> 
> Diffs
> -
> 
>   src/module/manager.hpp 302eb409fb8ef53b9cef8d2ecbe7b7f452b095ef 
>   src/tests/module.hpp 0820978441aede18dae6d1701433bff705b8c3c2 
> 
> Diff: https://reviews.apache.org/r/38627/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 37813: Added additional diagnostic output when a fetcher cache test fails.

[Bernd Mathiske]

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

(Updated Oct. 13, 2015, 9:27 a.m.)


Review request for mesos, Benjamin Hindman, Ben Mahler, Joseph Wu, and Till 
Toenshoff.


Changes
---

Now using HasFatalFailure() thanks to a tip from @bmahler, which simplifies the 
code a lot. Also addressed Till's issue.


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


Repository: mesos


Description
---

Dumps all involved task/executor sandbox contents in test tear down
only if a failure occurred.


Diffs (updated)
-

  src/tests/fetcher_cache_tests.cpp 7e194dc6e2b2d8b857e61d1a18d696545a86ce9f 

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


Testing
---

make check on OSX, where the bug showed up.


Thanks,

Bernd Mathiske

Re: Review Request 38564: Add a new callback enabling custom attribute discovery logic

[Guangya Liu]

> On 十月 13, 2015, 2:52 a.m., Guangya Liu wrote:
> > src/slave/slave.cpp, line 394
> > 
> >
> > I have one question for this after second review: I see that in 
> > slave.cpp, most APIs are calling HookManager::hooksAvailable() to check if 
> > there are hook managers. The problem is that if I have two hooks hookA and 
> > hookB, then the HookManager::hooksAvailable() will return True but the call 
> > in this code block is calling hookC which will be failed.
> > 
> > Do we need to enhance HookManager::hooksAvailable() by adding a 
> > parameter to check the availability for a specified hook manager?
> 
> Niklas Nielsen wrote:
> It is a best-effort short circuit: we know for sure that there won't be 
> anything to call if no hooks have been supplied. The hookC in your example 
> will have a default implementation and be a noop if not supplied by the user.
> 
> Does this make sense, or did you think of something else?
> If so, would you mind if we drop the issue here?

Hi Niklas, my orignal thinking is that we can enhance the API to 
HookManager::hooksAvailable(const string& hook), but the slave do not know each 
hook's real function, so we cannot check this in slave. Yes, your explanation 
does makse sense, thanks! ;-)


- Guangya


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


On 十月 13, 2015, 1:39 a.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38564/
> ---
> 
> (Updated 十月 13, 2015, 1:39 a.m.)
> 
> 
> Review request for mesos, Connor Doyle and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a new callback enabling custom attribute discovery logic
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 0c1042a 
>   src/examples/test_hook_module.cpp cd7c184 
>   src/hook/manager.hpp 3af1ff8 
>   src/hook/manager.cpp 108bd46 
>   src/slave/slave.cpp 01c5e42 
>   src/tests/hook_tests.cpp b35ce72 
> 
> Diff: https://reviews.apache.org/r/38564/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Re: Review Request 37813: Added additional diagnostic output when a fetcher cache test fails.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [37813]

All tests passed.

- Mesos ReviewBot


On Oct. 13, 2015, 4:27 p.m., Bernd Mathiske wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37813/
> ---
> 
> (Updated Oct. 13, 2015, 4:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Joseph Wu, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-3235
> https://issues.apache.org/jira/browse/MESOS-3235
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Dumps all involved task/executor sandbox contents in test tear down
> only if a failure occurred.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_cache_tests.cpp 7e194dc6e2b2d8b857e61d1a18d696545a86ce9f 
> 
> Diff: https://reviews.apache.org/r/37813/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX, where the bug showed up.
> 
> 
> Thanks,
> 
> Bernd Mathiske
> 
>

Re: Review Request 38869: Added static->dynamic transformation to Allocator.

[Michael Park]

> On Oct. 13, 2015, 5:20 p.m., Michael Park wrote:
> > All nits.
> > 
> > ```
> > make check -j 4 GTEST_FILTER=""  1984.45s user 77.00s system 367% cpu 
> > 9:20.95 total
> > make check -j 4 GTEST_FILTER=""  30.08s user 8.57s system 90% cpu 42.577 
> > total
> > ```

Oops. The `make check` numbers represent the compilation times for when a 
change is made in the implementation.

(1) Before: Added a comment in the function body in `hierarchical.hpp`.
(2) After: Added a comment in the function body in `hierarchical.cpp`.


- Michael


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


On Oct. 13, 2015, 8:51 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Oct. 13, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, Joseph 
> Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/master/allocator/mesos/hierarchical.hpp 
> d57c55e2f173c3054a04322a33ba590c67b82e4e 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

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/#review102443
---


The documentation added in this patch is great! Also consider breaking it out 
to a separate patch to separate behavioral vs non-behavioral changes. It really 
helps whoever's reviewing to stay within a narrower context :)


include/mesos/resources.hpp (lines 69 - 70)


`s/name and role/name, value, and role/`



include/mesos/resources.hpp (lines 75 - 76)


`s/parsing is successful/parsing was successful/`
`s/an Error if it isn't/an Error otherwise/`



include/mesos/resources.hpp (lines 92 - 93)


Same comment as above.



include/mesos/v1/resources.hpp (lines 350 - 414)


Could you explain why these can't simply live in `resources.cpp` as 
implementation details? Do we actually refer to private parts of the `Resource` 
object within these functions?



src/common/resources.cpp (line 361)


I think it would be better to only support one way of doing things here. I 
would opt for the array format. Is supporting both formats explicitly a feature?



src/common/resources.cpp (lines 392 - 393)


Why the wrapping here?



src/common/resources.cpp (lines 515 - 516)


This fits in one line now.



src/common/resources.cpp (lines 601 - 602)


This fits in one line now.



src/common/resources.cpp (line 1156)


Why the move here?


- Michael Park


On Oct. 12, 2015, 6:38 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 12, 2015, 6:38 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 38869: Added static->dynamic transformation to Allocator.

[Michael Park]

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

Ship it!


All nits.

```
make check -j 4 GTEST_FILTER=""  1984.45s user 77.00s system 367% cpu 9:20.95 
total
make check -j 4 GTEST_FILTER=""  30.08s user 8.57s system 90% cpu 42.577 total
```


src/master/allocator/mesos/hierarchical.hpp (line 80)


`s/internal:://`



src/master/allocator/mesos/hierarchical.hpp (line 99)


`s/internal:://`



src/master/allocator/mesos/hierarchical.hpp (line 389)


`s/internal:://`



src/master/allocator/mesos/hierarchical.cpp (line 53)


Not yours, but `s/Filter:/Filter :/`



src/master/allocator/mesos/hierarchical.cpp (line 96)


Not yours, but `s/Filter:/Filter :/`



src/master/allocator/mesos/hierarchical.cpp (lines 112 - 113)


This now fits in one line. Here and below.


- Michael Park


On Oct. 13, 2015, 8:51 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Oct. 13, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, Joseph 
> Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/master/allocator/mesos/hierarchical.hpp 
> d57c55e2f173c3054a04322a33ba590c67b82e4e 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 39211: Check for trailing characters in JSON::parse().

[Greg Mann]

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

(Updated Oct. 13, 2015, 5:51 p.m.)


Review request for mesos, Adam B and Joseph Wu.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

Check for trailing characters in JSON::parse().


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/json.hpp 
0870300c2d372722b4850155cb2a8e848b986ed5 
  3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 
b95667f4c1a1c170016da024a5e4af02ccaa064e 
  3rdparty/libprocess/3rdparty/stout/tests/json_tests.cpp 
131d39b87e571f16c774a133eb3148e98d8b9dd4 

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


Testing
---

Added tests to make sure that JSON::parse() is successfully returning errors 
when passed invalid JSON objects. Ran `make check`.


Thanks,

Greg Mann

Re: Review Request 39015: RegistryClient refactor: expanded abbreviated names.

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:15 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


Repository: mesos


Description
---

RegistryClient refactor: expanded abbreviated names.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39017: RegistryClient refactor: encapsulated Manifest

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:16 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


Repository: mesos


Description
---

Wrapped Manifest with ManifestResponse. getManifest returns the encapsulated 
ManifestResponse.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

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

[Greg Mann]

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

(Updated Oct. 13, 2015, 7:34 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael Park.


Changes
---

Addressed comments.


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 (updated)
-

  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 39218: Windows: Added support for `process/address.hpp`.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Oct. 13, 2015, 11:35 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39218/
> ---
> 
> (Updated Oct. 13, 2015, 11:35 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3628, MESOS-3629, MESOS-3630, MESOS-3631, and MESOS-3693
> 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-3693
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `process/address.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/address.hpp 
> bf230ac1a401284f4d4abdbaa53f5b8b9d83c000 
> 
> Diff: https://reviews.apache.org/r/39218/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 39265: Windows: Move `os::environment` to its own file.

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39019, 39076, 39091, 39092, 39093, 39096, 39097, 39180, 
39181, 39201, 39182, 39202, 39203, 39204, 39207, 39208, 39209, 39210, 39213, 
39217, 39218, 39219, 39220, 39262, 39263, 39264, 39265]

All tests passed.

- Mesos ReviewBot


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 39155: RegistryClient refactor: removed nested namespace references

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:17 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


Repository: mesos


Description
---

RegistryClient refactor: removed nested namespace references


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39112: RegistryClient refactor: fixed variable names

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:17 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


Repository: mesos


Description
---

RegistryClient refactor: fixed variable names. This patch will serve as 
catch-all for any variable name related changes in the refactor.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39053: RegistryClient refactor: priv method const'ness

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:16 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


Repository: mesos


Description
---

RegistryClient refactor: priv method const'ness


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39220: CMake: Added `slave/state.cpp` to Windows builds.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Oct. 13, 2015, 11:36 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39220/
> ---
> 
> (Updated Oct. 13, 2015, 11:36 a.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
> ---
> 
> CMake: Added `slave/state.cpp` to Windows builds.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt a5a66c13496a95bb7095504a4645909d887fb49f 
>   src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
> 
> Diff: https://reviews.apache.org/r/39220/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`.

[Joseph Wu]

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



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


Did you mean "non-OSX *and non-windows* platforms"?



3rdparty/libprocess/3rdparty/stout/include/stout/os/raw/environment.hpp (lines 
34 - 37)


Which header defines the `environ` macro on Windows?  And why isn't it 
included in this file?  (Or does it need to be included at all?)


- Joseph Wu


On Oct. 13, 2015, 11:36 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39262/
> ---
> 
> (Updated Oct. 13, 2015, 11:36 a.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 
> e6d36ec1bf414b52d0899f0edf83e0ad8910dd0e 
> 
> 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 39264: Windows:[3/3] Transitioned Mesos tests to use `os::raw` family.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Oct. 13, 2015, 11:36 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39264/
> ---
> 
> (Updated Oct. 13, 2015, 11:36 a.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.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Oct. 13, 2015, 11:36 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39263/
> ---
> 
> (Updated Oct. 13, 2015, 11:36 a.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 39014: RegistryClient refactor: renamed ManifestResponse

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:18 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


Repository: mesos


Description
---

RegistryClient refactor: renamed ManifestResponse as per review comments.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38579: Refactored registry client

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:18 p.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

rebased


Repository: mesos


Description
---

- Cleanup and broke big methods into smaller chunks.


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38747: Adding digest utilities

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:21 p.m.)


Review request for mesos, Ben Mahler, Gilbert Song, and Timothy Chen.


Repository: mesos


Description
---

Added SHA256, SHA512 implementation.


Diffs (updated)
-

  3rdparty/libprocess/Makefile.am eb7393904988afc0bc5e9f7dadd545e0d6c04e5f 
  3rdparty/libprocess/include/Makefile.am 
fcc62e99b92b9d2e7ab344e561a06dd6de1fef7e 
  3rdparty/libprocess/include/process/digest.hpp PRE-CREATION 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
a14b5b8fe22d3e75bef3c716ae7865ddaf132927 
  3rdparty/libprocess/src/tests/digest_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

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

[Joseph Wu]

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



3rdparty/libprocess/3rdparty/stout/include/Makefile.am (line 67)


Nit: We want the trailing slashes to line up on RB.



3rdparty/libprocess/3rdparty/stout/include/Makefile.am (line 96)


Nit: We want the trailing slashes to line up on RB.



3rdparty/libprocess/3rdparty/stout/include/stout/os/chsize.hpp (lines 21 - 23)


Can you rename the files/methods?

`chsize` seems to be the Windows name.  But `ftruncate` seems to better 
describe what the method is doing.



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/chsize.hpp (lines 24 
- 25)


This doc says that `ftruncate` needs an open fd too:
http://pubs.opengroup.org/onlinepubs/7908799/xsh/ftruncate.html


- Joseph Wu


On Oct. 13, 2015, 11:35 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39217/
> ---
> 
> (Updated Oct. 13, 2015, 11:35 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `stout/os/chsize.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/chsize.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/chsize.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/chsize.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
> 
>

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

[Alex Clemmer]

> On Oct. 13, 2015, 7:38 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/chsize.hpp, lines 
> > 24-25
> > 
> >
> > This doc says that `ftruncate` needs an open fd too:
> > http://pubs.opengroup.org/onlinepubs/7908799/xsh/ftruncate.html

Oh. Yes, sorry, I misread the comments of an important OSS codebase that ported 
`ftruncate`. Basically, what they said was that there is no equivalent of 
`truncate`, so you have to open the file to truncate it. They did not say, as I 
had thought, that the file descriptor doesn't need to be open on POSIX. My 
mistake. Of course in retrospect it has to be an open file descriptor.


- Alex


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


On Oct. 13, 2015, 6:35 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39217/
> ---
> 
> (Updated Oct. 13, 2015, 6:35 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `stout/os/chsize.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/chsize.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/chsize.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/chsize.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
> 
>

Re: Review Request 39068: RegistryClient refactor: Renamed fsLayerInfoList

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:15 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


Repository: mesos


Description
---

RegistryClient refactor: renamed fsLayerInfoList


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38941: Moved structs outside RegistryClient

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:15 p.m.)


Review request for mesos, Ben Mahler and Timothy Chen.


Changes
---

rebased


Repository: mesos


Description
---

Moved:
  - ManifestResponse
  - FilesystemLayerInfo


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39156: RegistryClient refactor: changed getManifest interface

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:18 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

rebased


Repository: mesos


Description
---

RegistryClient refactor: changed getManifest interface


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 38580: Added docker registry RemotePuller

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:19 p.m.)


Review request for mesos, Jie Yu, Timothy Chen, and Jiang Yan Xu.


Changes
---

rebased


Repository: mesos


Description
---

Integrated remote puller with store. Tests will follow.


Diffs (updated)
-

  src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
  src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
  src/slave/containerizer/provisioner/docker/local_puller.hpp 
4574e8a04663482625d7b54f765741f221ec13e0 
  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/registry_client.hpp 
fdb68b675582f603ffb3e96f31c1c9405e238567 
  src/slave/containerizer/provisioner/docker/registry_client.cpp 
4931ae8869a697b1e9d8d4cbc0a871e7cd506285 
  src/slave/containerizer/provisioner/docker/remote_puller.hpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 
  src/slave/containerizer/provisioner/docker/store.cpp 
637c97c0973bda492826803a962c99635647692d 
  src/slave/flags.hpp f76f0f6a0c7a01c7f061a225d7f6ef52be0ee7b5 
  src/slave/flags.cpp 029aa1eb00e1aa3e92c2155925022c17fd905862 
  src/tests/containerizer/provisioner_docker_tests.cpp 
d895eb9d0723e52cff8b21ef2deeaef1911d019c 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 39250: Puller refactor: moved untar to a common place

[Jojy Varghese]

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

(Updated Oct. 13, 2015, 7:19 p.m.)


Review request for mesos and Timothy Chen.


Changes
---

formatting


Repository: mesos


Description
---

Puller refactor: moved untar to a common place


Diffs (updated)
-

  src/slave/containerizer/provisioner/docker/local_puller.cpp 
74d0e1ead7d630e65a7e75cb6123139b9197efef 
  src/slave/containerizer/provisioner/docker/puller.hpp 
105b4e75439c2ad4c08e2fd364f288f1d39b9b59 
  src/slave/containerizer/provisioner/docker/puller.cpp 
cb05324689ffa26ce830b513e2d71b55517da3cb 
  src/slave/containerizer/provisioner/docker/remote_puller.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

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

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [39211, 39018]

All tests passed.

- Mesos ReviewBot


On Oct. 13, 2015, 6:30 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 13, 2015, 6:30 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 39018: Added JSON parsing for Resources.

[Greg Mann]

> On Oct. 13, 2015, 5 p.m., Michael Park wrote:
> > src/common/resources.cpp, line 361
> > 
> >
> > I think it would be better to only support one way of doing things 
> > here. I would opt for the array format. Is supporting both formats 
> > explicitly a feature?

No, it hasn't been explicitly called out as a feature. Why the desire to allow 
only one format? My thought was that any valid JSON string describing one or 
more Resource objects should return successfully, but I suppose there isn't too 
much need to accommodate a single resource object, since at the command line 
users will always be specifying multiple resources.


> On Oct. 13, 2015, 5 p.m., Michael Park wrote:
> > src/common/resources.cpp, line 1198
> > 
> >
> > Why the move here?

I was moving all of the private methods into an appropriately-labeled section. 
I think I will move some of the previous methods into the implementation as you 
suggested, so perhaps I will just return this instance of `find()` to its 
original location.


- Greg


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


On Oct. 13, 2015, 7:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 13, 2015, 7:34 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 39265: Windows: Move `os::environment` to its own file.

[Joseph Wu]

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

Ship it!


Ship It!

- Joseph Wu


On Oct. 13, 2015, 11:37 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39265/
> ---
> 
> (Updated Oct. 13, 2015, 11:37 a.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 39219: Windows: Added support for `slave/state.cpp`.

[Joseph Wu]

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

Ship it!



src/slave/state.cpp (line 648)


Per a comment in a previous review:
s/chsize/ftruncate/



src/slave/state.cpp (line 728)


Per a comment in a previous review:
s/chsize/ftruncate/


- Joseph Wu


On Oct. 13, 2015, 11:35 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39219/
> ---
> 
> (Updated Oct. 13, 2015, 11:35 a.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 39099: Changed secret field in Credential from 'bytes' to 'string' for V1

[Michael Park]

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

Ship it!


Ship It!

- Michael Park


On Oct. 13, 2015, 8 a.m., Isabel Jimenez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39099/
> ---
> 
> (Updated Oct. 13, 2015, 8 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change is introduced in dependant patch and we should update V1 proto as 
> well.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.proto eadbc9d 
> 
> Diff: https://reviews.apache.org/r/39099/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Isabel Jimenez
> 
>

Re: Review Request 39202: CMake: Moved libevent, gmock, http-parser to CMake on Windows.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:28 p.m.)


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


Repository: mesos


Description
---

CMake: Moved libevent, gmock, http-parser to CMake on Windows.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
f0de224b0d7924344fb1945b387b728a7241df87 
  3rdparty/libprocess/3rdparty/http-parser/CMakeLists.txt.template PRE-CREATION 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
ee1c74d31e28136bf289f4100d79a8ce568cd3af 
  3rdparty/libprocess/cmake/ProcessConfigure.cmake 
60108ff598fb075584196aa3d8e8e66e726c9f2a 
  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
ea6db09e1a1aa01450aee93814e07f09feae7ac9 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39201: Included `stout/check.hpp` in `future.hpp`.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:28 p.m.)


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


Repository: mesos


Description
---

The Windows integration work will require us to support `stout/net.hpp`,
but this file includes `stout/os.hpp`, which would be onerous to port.

To avoid having to port that entire file, we simply roped in what we
needed from `stout/net.hpp`. A consequence of unincluding os.hpp is that
`process/future.hpp` now requires a direct inclusion of
`stout/check.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/future.hpp 
9006b8a83d03eab6e67de12a954110029b7d150e 

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


Testing
---


Thanks,

Alex Clemmer

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

[Greg Mann]

> On Oct. 13, 2015, 3:35 a.m., Guangya Liu wrote:
> > include/mesos/resources.hpp, lines 114-118
> > 
> >
> > Can this be removed or merged to the under comments?

I put the comment here so that it wouldn't interfere with the Doxygen 
documentation, which should be placed directly before the member declaration. I 
don't want to remove it, because I think the TODO is still valid. I'm open to 
moving it if there is a better place for it; do you have one in mind?


> On Oct. 13, 2015, 3:35 a.m., Guangya Liu wrote:
> > include/mesos/resources.hpp, line 120
> > 
> >
> > s/Validates/Validate

This description should be fine; see the Google C++ style guide, which states 
that function comments should be "descriptive" rather than "declarative": 
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Comments


> On Oct. 13, 2015, 3:35 a.m., Guangya Liu wrote:
> > include/mesos/resources.hpp, line 412
> > 
> >
> > s/conflictingTypes/conflictTypes?

I would suggest either `conflictingTypes` or `typesConflict`, since these are 
both abbreviations of English sentences that describe what the method does, 
i.e. it "checks for conflicting types", or "checks to see if any types 
conflict". I don't like `conflictTypes` because it sounds like the method will 
cause a conflict of some kind. I like the current name, but am open to changing 
it if there is a better alternative.


- Greg


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


On Oct. 12, 2015, 6:38 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 12, 2015, 6:38 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 39204: Windows: Added support for `stout/os/read.hpp`.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:29 p.m.)


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


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


Repository: mesos


Description
---

Windows: Added support for `stout/os/read.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/read.hpp 
ffacce164980157ea225a4e64e33b8bf8ec38ab7 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/read.hpp 
d5698a5b44fd6083ac3119d6825d31f46efb2f38 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/read.hpp 
09d63329f16f13d408742f9fc8f596d76c4d70c9 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
50e35f43d87c69a83a9e7d039d1881404ea8be38 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39210: Windows: Moved `realpath` to its own file, `stout/os/realpath.hpp`.

[Alex Clemmer]

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

(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 `realpath` to its own file, `stout/os/realpath.hpp`.

(NOTE: This does not resolve MESOS-3632, it's only a partial solution.)


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
5141c1369af60afd6cd5c70c6295d575ea960a83 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/realpath.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp 
ddcda7bdb294272a7a64a7a46e0576e8c4430eec 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39209: Windows: Move ::UUID to stout::UUID to avoid namespace collision.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:33 p.m.)


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


Repository: mesos


Description
---

The `Windows.h` head includes a header that defines the UUID struct for
the DCE RPC API. This is dumped into the global namespace, just like our
UUID implementation found in `stout/uuid.hpp`.

Since this causes a compilation error on Windows, we simply move our
UUID into `stout::`. We further add a `using stout::UUID;` in the file
so that we don't have to change every callsite in the Mesos codebase
that uses it.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/uuid.hpp 
bc167f1fa9e64f89138f131e726e7733c66da29c 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39208: Windows: Add windows support to `stout/protobuf.hpp`.

[Joseph Wu]

> On Oct. 12, 2015, 2:13 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 41-42
> > 
> >
> > Do you want to `#include ` too?
> 
> Alex Clemmer wrote:
> Interesting, you must be seeing good reasons to do this that I haven't 
> seen yet. Is it just to make the header self-contained? Is that what best 
> practice is here? (I'm pretty much 100% a C++ noob.)

Yup.  If we (for some inexplicable reason) change open.hpp to *not* include 
close.hpp, we wouldn't want this file to change if that happens.


- Joseph


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


On Oct. 13, 2015, 11:31 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39208/
> ---
> 
> (Updated Oct. 13, 2015, 11:31 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3634
> https://issues.apache.org/jira/browse/MESOS-3634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Add windows support to `stout/protobuf.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 2285ce9eaba668d5215c108849055fe92163da4d 
> 
> Diff: https://reviews.apache.org/r/39208/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 38564: Add a new callback enabling custom attribute discovery logic

[Niklas Nielsen]

> On Oct. 12, 2015, 7:52 p.m., Guangya Liu wrote:
> > src/slave/slave.cpp, line 394
> > 
> >
> > I have one question for this after second review: I see that in 
> > slave.cpp, most APIs are calling HookManager::hooksAvailable() to check if 
> > there are hook managers. The problem is that if I have two hooks hookA and 
> > hookB, then the HookManager::hooksAvailable() will return True but the call 
> > in this code block is calling hookC which will be failed.
> > 
> > Do we need to enhance HookManager::hooksAvailable() by adding a 
> > parameter to check the availability for a specified hook manager?

It is a best-effort short circuit: we know for sure that there won't be 
anything to call if no hooks have been supplied. The hookC in your example will 
have a default implementation and be a noop if not supplied by the user.

Does this make sense, or did you think of something else?
If so, would you mind if we drop the issue here?


- Niklas


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


On Oct. 12, 2015, 6:39 p.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38564/
> ---
> 
> (Updated Oct. 12, 2015, 6:39 p.m.)
> 
> 
> Review request for mesos, Connor Doyle and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a new callback enabling custom attribute discovery logic
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 0c1042a 
>   src/examples/test_hook_module.cpp cd7c184 
>   src/hook/manager.hpp 3af1ff8 
>   src/hook/manager.cpp 108bd46 
>   src/slave/slave.cpp 01c5e42 
>   src/tests/hook_tests.cpp b35ce72 
> 
> Diff: https://reviews.apache.org/r/38564/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Re: Review Request 38564: Add a new callback enabling custom attribute discovery logic

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On 十月 13, 2015, 1:39 a.m., Felix Abecassis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38564/
> ---
> 
> (Updated 十月 13, 2015, 1:39 a.m.)
> 
> 
> Review request for mesos, Connor Doyle and Niklas Nielsen.
> 
> 
> Bugs: MESOS-3366
> https://issues.apache.org/jira/browse/MESOS-3366
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add a new callback enabling custom attribute discovery logic
> 
> 
> Diffs
> -
> 
>   include/mesos/hook.hpp 0c1042a 
>   src/examples/test_hook_module.cpp cd7c184 
>   src/hook/manager.hpp 3af1ff8 
>   src/hook/manager.cpp 108bd46 
>   src/slave/slave.cpp 01c5e42 
>   src/tests/hook_tests.cpp b35ce72 
> 
> Diff: https://reviews.apache.org/r/38564/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Felix Abecassis
> 
>

Re: Review Request 39210: Windows: Moved `realpath` to its own file, `stout/os/realpath.hpp`.

[Joseph Wu]

> On Oct. 12, 2015, 2:29 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/realpath.hpp, line 25
> > 
> >
> > Don't you need to include `windows.hpp` to get `PATH_MAX` (on Windows, 
> > of course)?
> 
> Alex Clemmer wrote:
> We don't need to because result.hpp includes abort.hpp which includes 
> windows.hpp.
> 
> That said, it might be better to include it here also. What do you think? 
> I don't see a clear advantage either way.

We generally try (emphasis on "try") to have a single level of include 
dependency in all our files.  So I'd vote to include it.


- Joseph


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


On Oct. 10, 2015, 5:23 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39210/
> ---
> 
> (Updated Oct. 10, 2015, 5:23 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 `realpath` to its own file, `stout/os/realpath.hpp`.
> 
> (NOTE: This does not resolve MESOS-3632, it's only a partial solution.)
> 
> 
> 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/realpath.hpp 
> PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/exists.hpp 
> ddcda7bdb294272a7a64a7a46e0576e8c4430eec 
> 
> Diff: https://reviews.apache.org/r/39210/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 39152: Pass LIBPROCESS_IP even when executor environment is specified.

[Niklas Nielsen]

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



src/slave/containerizer/containerizer.cpp (line 256)


Shouldn't we only set this if it is not present?


- Niklas Nielsen


On Oct. 9, 2015, 1:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39152/
> ---
> 
> (Updated Oct. 9, 2015, 1:34 p.m.)
> 
> 
> Review request for mesos, Adam B and Kapil Arya.
> 
> 
> Bugs: MESOS-3553
> https://issues.apache.org/jira/browse/MESOS-3553
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If DNS is not available on the agent node and a task is launched which 
> explicitly specifies the executor's environment, LIBPROCESS_IP will not be 
> passed through and the default hostname lookup after spawning the executor 
> process will throw an error. This patch alters the agent to always pass 
> LIBPROCESS_IP, even when the executor environment is specified.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> 25c87e9f948b7efe8b9a853c403bee69982d6c4c 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 5bc7d408bda0c249e1b66747d8bd87e688362e6c 
> 
> Diff: https://reviews.apache.org/r/39152/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 39208: Windows: Add windows support to `stout/protobuf.hpp`.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:31 p.m.)


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


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


Repository: mesos


Description
---

Windows: Add windows support to `stout/protobuf.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
2285ce9eaba668d5215c108849055fe92163da4d 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39207: Windows: Move `write` to its own file, `stout/os/write.hpp`.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:30 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: Move `write` to its own file, `stout/os/write.hpp`.

(This part isn't going in the commit message, but just FYI, we're doing this 
(1) because we need it to support `slave/state.hpp`, (2) because we don't want 
to port all of `os.hpp` to get this one function, and (3) because 
`stout/os/read.hpp` already exists, so it makes sense to have one for `write` 
as well.)

(Also, please do not close MESOS-3632, this is just a partial solution.)


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
5141c1369af60afd6cd5c70c6295d575ea960a83 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/write.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
a042d061159c83e1652e7288b90809981d2818c9 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
2b0966889af73238a08e29f1136d0ce286a0ffda 

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


Testing
---

Ran `make check` on Windows 10, OS X 10.10, Ubuntu 15. On Ubuntu 15, we also 
checked that the autotools solution builds and the tests run as well.


Thanks,

Alex Clemmer

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

[Alex Clemmer]

---
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 (updated)
-

  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 39217: Windows: Added `stout/os/chsize.hpp`.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:35 p.m.)


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


Repository: mesos


Description
---

Windows: Added `stout/os/chsize.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/chsize.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/chsize.hpp 
PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/chsize.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

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. 13, 2015, 6:35 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 (updated)
-

  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 38869: Added static->dynamic transformation to Allocator.

[Ben Mahler]

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


Thanks for doing this! One minor issue about the use of the factory stuff below.


src/master/allocator/mesos/hierarchical.hpp (lines 65 - 91)


Do we need all of this factory stuff here? Any reason we can't just pass in 
std::functions as the factories instead of having the polymorphism indirection 
here?


- Ben Mahler


On Oct. 13, 2015, 8:51 a.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38869/
> ---
> 
> (Updated Oct. 13, 2015, 8:51 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Cody Maloney, Artem Harutyunyan, Joseph 
> Wu, and Michael Park.
> 
> 
> Bugs: MESOS-3554
> https://issues.apache.org/jira/browse/MESOS-3554
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This improves the compilation time of Mesos significantly, allowing
> developers to iterate more quickly on allocator changes.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 536a99f3dc2f48e6002e70b605598f4d218a4324 
>   src/Makefile.am d855cb83277c3e0e2ee3feacaf6ad0962223ef6e 
>   src/master/allocator/mesos/hierarchical.hpp 
> d57c55e2f173c3054a04322a33ba590c67b82e4e 
>   src/master/allocator/mesos/hierarchical.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/38869/diff/
> 
> 
> Testing
> ---
> 
> make check
> touched hierarchical.cpp and recompiled. Verified we only rebuild the module 
> and relink.
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 39097: CMake:[2/2] remove `__WINDOWS__` flag definition from Stout config.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:26 p.m.)


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


Repository: mesos


Description
---

CMake:[2/2] remove `__WINDOWS__` flag definition from Stout config.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
c4d1b7b3f896f8037d922fafcdbdd0e31a70e140 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39180: Windows: Added support for `stout/os/open.hpp`.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:26 p.m.)


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


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


Repository: mesos


Description
---

Windows: Added support for `stout/os/open.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/open.hpp 
43f261fd7a60b534f642f826ebf6ab18d72180c2 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/open.hpp 
023993d859e3a101ca387c1a514cd75de0d2beb1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/open.hpp 
14fa11765c222cb4e80f5e45360d0f05facb578e 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
50e35f43d87c69a83a9e7d039d1881404ea8be38 

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


Testing
---

Ran both stout_tests from VS.


Thanks,

Alex Clemmer

Re: Review Request 39096: CMake:[1/2] Moved `__WINDOWS__` flag definition to CompilationConfigure.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:26 p.m.)


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


Repository: mesos


Description
---

CMake:[1/2] Moved `__WINDOWS__` flag definition to CompilationConfigure.


Diffs (updated)
-

  cmake/CompilationConfigure.cmake d1d75098d0ad655a7dcd74628bffd7ba8547b454 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39093: CMake:[3/3] Add `make check` target.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:25 p.m.)


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


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


Repository: mesos


Description
---

CMake:[3/3] Add `make check` target.


Diffs (updated)
-

  CMakeLists.txt a5a66c13496a95bb7095504a4645909d887fb49f 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39181: Windows: Added support for `stout/net.hpp`.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:27 p.m.)


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


Bugs: MESOS-3630 and MESOS-3631
https://issues.apache.org/jira/browse/MESOS-3630
https://issues.apache.org/jira/browse/MESOS-3631


Repository: mesos


Description
---

Windows: Added support for `stout/net.hpp`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
9e9c3119ad18f4cbc70c70095c71dc4fd19553df 
  3rdparty/libprocess/3rdparty/stout/include/stout/net.hpp 
3f829bafe96526bc2263c9f228f85de38c435f60 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/net.hpp 
11e3895ee46e36faca0d2e1b436b61576826e472 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/net.hpp 
4f82796c2b9ef6a9198be145d969c5fce933be49 

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


Testing
---

Ran stout tests from VS.


Thanks,

Alex Clemmer

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

[Greg Mann]

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

(Updated Oct. 13, 2015, 6:30 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Jie Yu, and Michael Park.


Changes
---

Addressed comments.


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 (updated)
-

  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 39092: CMake:[2/3] prepared process library tests to run with `make check`.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:25 p.m.)


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


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


Repository: mesos


Description
---

Right now running the Mesos tests from the default tests target will (1)
aggregate all our tests into a semi-useless report that hides all the
errors, and (2) strip all our colors when you do pass the verbosity flag
to get rid of the "report" structure.

This commit will prepare us to run the tests with with color, without
this default reporting structure. We do this by moving the tests target
to a `CACHE STRING` which can be referenced by the tests.


Diffs (updated)
-

  3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
ea6db09e1a1aa01450aee93814e07f09feae7ac9 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
a14b5b8fe22d3e75bef3c716ae7865ddaf132927 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39208: Windows: Add windows support to `stout/protobuf.hpp`.

[Alex Clemmer]

> On Oct. 12, 2015, 9:13 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp, lines 41-42
> > 
> >
> > Do you want to `#include ` too?

Interesting, you must be seeing good reasons to do this that I haven't seen 
yet. Is it just to make the header self-contained? Is that what best practice 
is here? (I'm pretty much 100% a C++ noob.)


- Alex


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


On Oct. 13, 2015, 6:31 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39208/
> ---
> 
> (Updated Oct. 13, 2015, 6:31 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Joris Van Remoortere, and Joseph 
> Wu.
> 
> 
> Bugs: MESOS-3634
> https://issues.apache.org/jira/browse/MESOS-3634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Add windows support to `stout/protobuf.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/protobuf.hpp 
> 2285ce9eaba668d5215c108849055fe92163da4d 
> 
> Diff: https://reviews.apache.org/r/39208/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

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

[Alex Clemmer]

---
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 (updated)
-

  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 39152: Pass LIBPROCESS_IP even when executor environment is specified.

[Greg Mann]

> On Oct. 13, 2015, 6:21 p.m., Niklas Nielsen wrote:
> > src/slave/containerizer/containerizer.cpp, line 256
> > 
> >
> > Shouldn't we only set this if it is not present?

Since it gets set before the `foreach()` loop below, if `LIBPROCESS_IP` is 
present in the passed flags, the passed value will overwrite the value that we 
set here.


- Greg


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


On Oct. 9, 2015, 8:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39152/
> ---
> 
> (Updated Oct. 9, 2015, 8:34 p.m.)
> 
> 
> Review request for mesos, Adam B and Kapil Arya.
> 
> 
> Bugs: MESOS-3553
> https://issues.apache.org/jira/browse/MESOS-3553
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If DNS is not available on the agent node and a task is launched which 
> explicitly specifies the executor's environment, LIBPROCESS_IP will not be 
> passed through and the default hostname lookup after spawning the executor 
> process will throw an error. This patch alters the agent to always pass 
> LIBPROCESS_IP, even when the executor environment is specified.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> 25c87e9f948b7efe8b9a853c403bee69982d6c4c 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 5bc7d408bda0c249e1b66747d8bd87e688362e6c 
> 
> Diff: https://reviews.apache.org/r/39152/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Alex Clemmer]

---
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 (updated)
-

  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 39263: Windows:[2/3] transitioned libprocess to use `os::raw::` family.

[Alex Clemmer]

---
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 (updated)
-

  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 39018: Added JSON parsing for Resources.

[Greg Mann]

> On Oct. 13, 2015, 5:38 a.m., Guangya Liu wrote:
> > src/common/resources.cpp, line 367
> > 
> >
> > Why not check error here?

I'm sorry, I don't understand this comment. Are you referring to the error 
checking for trailing characters?


- Greg


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


On Oct. 12, 2015, 6:38 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 12, 2015, 6:38 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 39076: CMake: Added ability of Windows builds to include protobuf headers.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:25 p.m.)


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


Repository: mesos


Description
---

CMake: Added ability of Windows builds to include protobuf headers.


Diffs (updated)
-

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
ee1c74d31e28136bf289f4100d79a8ce568cd3af 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 39091: CMake:[1/3] Prepared stout tests to run with `make check`.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:25 p.m.)


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


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


Repository: mesos


Description
---

Right now running the Mesos tests from the default tests target will
(1) aggregate all our tests into a semi-useless report that hides all
the errors, and (2) strip all our colors when you do pass the verbosity
flag to get rid of the "report" structure.

This commit will prepare us to run the tests with with color, without
this default reporting structure. We do this by moving the tests target
to a `CACHE STRING` which can be referenced by the tests.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/cmake/StoutTestsConfigure.cmake 
c4d1b7b3f896f8037d922fafcdbdd0e31a70e140 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
94292f8a46ec31bbaf6e52f48109322bbe123f70 

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


Testing
---


Thanks,

Alex Clemmer

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

[Michael Park]

> On Oct. 13, 2015, 5:38 a.m., Guangya Liu wrote:
> > src/common/resources.cpp, line 367
> > 
> >
> > Why not check error here?
> 
> Greg Mann wrote:
> I'm sorry, I don't understand this comment. Are you referring to the 
> error checking for trailing characters?

This is because the result is already of type `Try`. No need to 
unwrap the content only to rewrap it into a `Try`, we can just 
propagate whatever the result is.


- Michael


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


On Oct. 12, 2015, 6:38 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39018/
> ---
> 
> (Updated Oct. 12, 2015, 6:38 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 39182: Windows: Enable ip_tests.

[Alex Clemmer]

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

(Updated Oct. 13, 2015, 6:27 p.m.)


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


Bugs: MESOS-3439 and MESOS-3629
https://issues.apache.org/jira/browse/MESOS-3439
https://issues.apache.org/jira/browse/MESOS-3629


Repository: mesos


Description
---

Windows: Enable ip_tests.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/duration.hpp 
90551541f59889e96b21dbe1b65d3904850464c2 
  3rdparty/libprocess/3rdparty/stout/include/stout/ip.hpp 
d1e2df6151149e03ffb4a76e2c24ff78b891e016 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
50e35f43d87c69a83a9e7d039d1881404ea8be38 
  3rdparty/libprocess/3rdparty/stout/tests/CMakeLists.txt 
94292f8a46ec31bbaf6e52f48109322bbe123f70 

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


Testing
---

Ran stout_tests from VS.


Thanks,

Alex Clemmer