Re: Review Request 47505: Updated authorizer.proto Subject, Object and Request.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47505]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 18, 2016, 12:35 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47505/
> ---
> 
> (Updated May 18, 2016, 12:35 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5405
> https://issues.apache.org/jira/browse/MESOS-5405
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Makes `value` required for clearity, to make sure that we do not
> introduce two ways of expressing `ANY`.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 7d4aa32f42de538864508a0ba481d875917d9ab9 
> 
> Diff: https://reviews.apache.org/r/47505/diff/
> 
> 
> Testing
> ---
> 
> make check (OSX)
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Review Request 47518: Added test case for no volume and no checkpoint.

[Guangya Liu]

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added test case for no volume and no checkpoint.


Diffs
-

  src/tests/containerizer/docker_volume_isolator_tests.cpp 
c524f42743bf08ee54f1cbb083d0d3c85a8b70c9 

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


Testing
---

make
make check

[ RUN  ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsWithoutVolumes
[   OK ] DockerVolumeIsolatorTest.ROOT_CommandTaskNoRootfsWithoutVolumes 
(384 ms)


Thanks,

Guangya Liu

Re: Review Request 36383: Updated docker volume test case by adding checkpoint verify logic.

[Guangya Liu]

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

(Updated 五月 18, 2016, 5:51 a.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Rebase


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


Repository: mesos


Description
---

Updated docker volume test case by adding checkpoint verify logic.


Diffs (updated)
-

  src/tests/containerizer/docker_volume_isolator_tests.cpp 
c524f42743bf08ee54f1cbb083d0d3c85a8b70c9 

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


Testing
---

make
make check
GLOG_v=1 ./bin/mesos-tests.sh  --gtest_filter="DockerVolumeIsolatorTest.*" 
--gtest_repeat=100


Thanks,

Guangya Liu

Re: Review Request 47463: Added documentation for `network/cni` isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47463]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 17, 2016, 11:27 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 11:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4771
> https://issues.apache.org/jira/browse/MESOS-4771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `network/cni` isolator.
> 
> 
> Diffs
> -
> 
>   docs/cni.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47463/diff/
> 
> 
> Testing
> ---
> 
> Build the documentation website and verified the rendering.
> 
> You can review a rendering of the markdown on my github:
> https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 47492: Windows: Updated the containerizer name to `mesos-containerizer.exe`.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47492]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 17, 2016, 9:33 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47492/
> ---
> 
> (Updated May 17, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Updated the containerizer name to `mesos-containerizer.exe`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
> 
> Diff: https://reviews.apache.org/r/47492/diff/
> 
> 
> Testing
> ---
> 
> Windows: build/run
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 46373: Request /files/read.json with a negative length value causes error.

[zhou xing]

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

(Updated 五月 18, 2016, 4:13 a.m.)


Review request for mesos, Benjamin Mahler and Greg Mann.


Changes
---

update comments


Bugs: mesos-5060
https://issues.apache.org/jira/browse/mesos-5060


Repository: mesos


Description
---

[MESOS-5060]
The patch did the following changes:
1. Fix the length logic in files.cpp.
2. Add some tests to test the /files/read.json endponit with
negative length.


Diffs (updated)
-

  src/files/files.cpp e4b0ada00aabba6553810391f4015a896f8a69a5 
  src/tests/files_tests.cpp 5d6620d13babaf0bb7f9c888bb1b4fa2228b6ccd 

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


Testing
---

make
make check

request 'files/read.json' endpoint with negative offset or length argument


Thanks,

zhou xing

Re: Review Request 46373: Request /files/read.json with a negative length value causes error.

[zhou xing]

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

(Updated 五月 18, 2016, 4:07 a.m.)


Review request for mesos, Benjamin Mahler and Greg Mann.


Changes
---

update code according to Greg's comments


Bugs: mesos-5060
https://issues.apache.org/jira/browse/mesos-5060


Repository: mesos


Description
---

[MESOS-5060]
The patch did the following changes:
1. Fix the length logic in files.cpp.
2. Add some tests to test the /files/read.json endponit with
negative length.


Diffs (updated)
-

  src/files/files.cpp e4b0ada00aabba6553810391f4015a896f8a69a5 
  src/tests/files_tests.cpp 5d6620d13babaf0bb7f9c888bb1b4fa2228b6ccd 

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


Testing
---

make
make check

request 'files/read.json' endpoint with negative offset or length argument


Thanks,

zhou xing

Re: Review Request 45605: Introduced HTB class.

[haosdent huang]

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




src/linux/routing/queueing/htb.cpp (line 49)


Why we pass `uint64_t` above and use `uint32_t` here?



src/linux/routing/queueing/htb.cpp (line 100)


```
return Error(string(nl_geterror(error)));
```

because `using std::string;` above.


- haosdent huang


On April 1, 2016, 9:50 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45605/
> ---
> 
> (Updated April 1, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Bugs: mesos-4749
> https://issues.apache.org/jira/browse/mesos-4749
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced HTB class API, prepare for per-container bandwidth limit.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/internal.hpp 8f68119819f7c79ece1a13ac1894b1802ddc8e19 
>   src/linux/routing/queueing/discipline.hpp 
> 54d6b214ef6a38fd8279f6d01e6f4e3ccfddf634 
>   src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
>   src/linux/routing/queueing/htb.cpp faadf32bd48cc6bf968b1229789903c0d01fd75c 
>   src/linux/routing/queueing/internal.hpp 
> 768ed325f9b259e150779eb3ad74f4e5d4bcc7a2 
> 
> Diff: https://reviews.apache.org/r/45605/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 46373: Request /files/read.json with a negative length value causes error.

[Greg Mann]

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




src/files/files.cpp (lines 374 - 376)


How about: "The pailer in the webui sends `length=-1` at first to determine 
the length of the file, so we allow a length of -1. Setting `length=-1` has the 
same effect as not providing a length: we read to the end of the file, up to 
the maximum read length."



src/files/files.cpp (lines 378 - 383)


What do you think about:
`if (result.get() < -1) { ... }`
`if (result.get() > -1) { ... }`

I think that might improve readability a bit?



src/tests/files_tests.cpp (lines 175 - 176)


How about: "The pailer in the webui will send `length=-1` at first to 
determine the length of the file, so we need to accept a length of -1."


- Greg Mann


On May 17, 2016, 5:29 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46373/
> ---
> 
> (Updated May 17, 2016, 5:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Greg Mann.
> 
> 
> Bugs: mesos-5060
> https://issues.apache.org/jira/browse/mesos-5060
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [MESOS-5060]
> The patch did the following changes:
> 1. Fix the length logic in files.cpp.
> 2. Add some tests to test the /files/read.json endponit with
> negative length.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp e4b0ada00aabba6553810391f4015a896f8a69a5 
>   src/tests/files_tests.cpp 5d6620d13babaf0bb7f9c888bb1b4fa2228b6ccd 
> 
> Diff: https://reviews.apache.org/r/46373/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> request 'files/read.json' endpoint with negative offset or length argument
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 47485: Added utility for parsing ld.so.cache on linux.

[Kevin Klues]

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

(Updated May 18, 2016, 3:40 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Forgot to update header when updating cpp file.


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


Repository: mesos


Description
---

Added utility for parsing ld.so.cache on linux.


Diffs (updated)
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
  src/linux/ldcache.hpp PRE-CREATION 
  src/linux/ldcache.cpp PRE-CREATION 
  src/tests/ldcache_tests.cpp PRE-CREATION 

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


Testing
---

GTEST_FILTER="LdcacheTest.Parse" make check -j


Thanks,

Kevin Klues

Re: Review Request 47481: Rewrote os::read() to be friendlier to reading binary data.

[Kevin Klues]

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

(Updated May 18, 2016, 3:25 a.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and switched 
to 'mcypark'.


Changes
---

Removed erroneous #include


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


Repository: mesos


Description
---

The previous read() implementation was based on calling getline() to
read in chunks of data from a file. This is fine for text-based files,
but is a little strange for binary files.

The new implementation reads in chunks of raw bytes into a stack
allocated buffer before copying them into their final location. I
usually don't like stack allocated buffers because of their potential
security implications when it comes to stack buffer overflows, but I
took extra care here to make sure there won't be any.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/read.hpp 
e1e97c1bcb7493a734fc77721a83c230b1a23724 

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


Testing
---

make check -j


Thanks,

Kevin Klues

Re: Review Request 47485: Added utility for parsing ld.so.cache on linux.

[Kevin Klues]

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

(Updated May 18, 2016, 3:21 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Updated based on changes to the ELF parsing utility.


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


Repository: mesos


Description
---

Added utility for parsing ld.so.cache on linux.


Diffs (updated)
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
  src/linux/ldcache.hpp PRE-CREATION 
  src/linux/ldcache.cpp PRE-CREATION 
  src/tests/ldcache_tests.cpp PRE-CREATION 

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


Testing
---

GTEST_FILTER="LdcacheTest.Parse" make check -j


Thanks,

Kevin Klues

Re: Review Request 47481: Rewrote os::read() to be friendlier to reading binary data.

[Kevin Klues]

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

(Updated May 18, 2016, 3:18 a.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and switched 
to 'mcypark'.


Changes
---

Updated to use a heap allocated buffer instead of a stack allocated one.


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


Repository: mesos


Description
---

The previous read() implementation was based on calling getline() to
read in chunks of data from a file. This is fine for text-based files,
but is a little strange for binary files.

The new implementation reads in chunks of raw bytes into a stack
allocated buffer before copying them into their final location. I
usually don't like stack allocated buffers because of their potential
security implications when it comes to stack buffer overflows, but I
took extra care here to make sure there won't be any.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/read.hpp 
e1e97c1bcb7493a734fc77721a83c230b1a23724 

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


Testing
---

make check -j


Thanks,

Kevin Klues

Re: Review Request 47482: Added preliminary support for parsing ELF files in stout.

[Kevin Klues]

> On May 17, 2016, 10:23 p.m., Jojy Varghese wrote:
> > 3rdparty/stout/include/stout/elf.hpp, line 77
> > 
> >
> > Dont we have to close the open file in error cases?

What I actually needed to be doing was deleting the File object on every error 
path.  This implicitly calls `close()` and `elf_end()` when needed as well as 
deletes the memory for the object. I was originally returning a `Try` 
rather than a `Try`, so this destruction  was happening automatically as 
the object went out of scope.  This has some obviousl problems of it's own, 
which is why I moved to the pointer model.

I really wanted to return a `Try>` here, but `Try` has 
problems with the missing copy consructor on `unique_ptr`, so I had to settle 
for a normal pointer.  Unfortunately, this means that I have to manually delete 
this object at the call site, but I don't think that's much of a problem in 
this particular case (we will very rarely be working with these objects).


> On May 17, 2016, 10:23 p.m., Jojy Varghese wrote:
> > 3rdparty/stout/include/stout/elf.hpp, line 94
> > 
> >
> > Maybe break out after finding DYNAMIC section?

Since there is only one valid `case` statement, a break is unnecessary. 
Moreover, as new `case` statements become valid, I don't actually envision them 
having a break at all, more like:
```
switch ((SectionType)shdr.sh_type) {
  case SectionType::TYPE1:
  case SectionType::TYPE2:
  case SectionType::TYPE3:
  case SectionType::DYNAMIC:
file->sections[(SectionType)shdr.sh_type].push_back(scn);
}
```


> On May 17, 2016, 10:23 p.m., Jojy Varghese wrote:
> > 3rdparty/stout/include/stout/elf.hpp, line 184
> > 
> >
> > emplace_back?

What's the advantage or disadvantage of this?  My impression is that 
`emplace_back()` is only really useful if you want to pass a bunch or arguments 
to it in the `...` style rather than passing it a single object or a whole 
nother vector.


> On May 17, 2016, 10:23 p.m., Jojy Varghese wrote:
> > 3rdparty/stout/include/stout/elf.hpp, line 196
> > 
> >
> > const?

Looking at this again, I don't even need to save the `path` as part of the 
`File` object.  I've removed it.


- Kevin


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


On May 17, 2016, 7:17 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47482/
> ---
> 
> (Updated May 17, 2016, 7:17 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5400
> https://issues.apache.org/jira/browse/MESOS-5400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Right now we are able to parse ELF formatted shared libraries and
> extract their canonical SONAME and external library dependencies. In
> the future, we should add support for fully parsing an ELf file for
> easy access to all of its contents.
> 
> The current implementation relies on libelf. We should probably remove
> this dependency in future versions (mostly since the headers for
> libelf are not installed on a standard Linux distribution by default).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/configure.ac ada1e22c72875fe9d557f07e4846128be0bcef13 
>   3rdparty/stout/include/stout/elf.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47482/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 47482: Added preliminary support for parsing ELF files in stout.

[Kevin Klues]

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

(Updated May 18, 2016, 3:16 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Updated based on comments.  Thanks!


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


Repository: mesos


Description
---

Right now we are able to parse ELF formatted shared libraries and
extract their canonical SONAME and external library dependencies. In
the future, we should add support for fully parsing an ELf file for
easy access to all of its contents.

The current implementation relies on libelf. We should probably remove
this dependency in future versions (mostly since the headers for
libelf are not installed on a standard Linux distribution by default).


Diffs (updated)
-

  3rdparty/stout/configure.ac ada1e22c72875fe9d557f07e4846128be0bcef13 
  3rdparty/stout/include/stout/elf.hpp PRE-CREATION 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 47499: Make Mesos site responsive.

[haosdent huang]

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




site/source/assets/css/main.css (line 125)


There is a responsive navbar in bootstrap 
http://getbootstrap.com/components/#navbar I think could adjust current code to 
use it.


- haosdent huang


On May 18, 2016, 12:24 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47499/
> ---
> 
> (Updated May 18, 2016, 12:24 a.m.)
> 
> 
> Review request for mesos, Tim Anderegg and Vinod Kone.
> 
> 
> Bugs: MESOS-5402
> https://issues.apache.org/jira/browse/MESOS-5402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wrap lines on small screens,
> so the page is more friendly to view on
> mobile.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
> 
> Diff: https://reviews.apache.org/r/47499/diff/
> 
> 
> Testing
> ---
> 
> Manually checked some random pages in different sizes.
> 
> * PC
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/pT87vto4CHMMaG2/old_pc.png)
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/idXPLIM8t0LxRWA/new_pc.png)
> * iPad
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/Wt3g8bknstahXSC/old_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/TBTjlj1xT3YFrOf/new_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/sMct2b54lfCzrAu/old_iPad.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/qp2hj1UCNOfGLIX/new_iPad.png)
> * iPhone
> ![iPhone](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/mCTMOx8FLFxANeo/old_iPhone.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/5hlKDsOKqdJaztw/new_iPhone.png)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47486: Windows: Escaped command line arguments.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47486]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 17, 2016, 7:19 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47486/
> ---
> 
> (Updated May 17, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
> Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The command line for the containerizer has the command encoded
> as JSON. Non escaped quotes are removed during the containerizer
> startup and the JSON processed is invalid.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/windows/subprocess.hpp 
> 551770b9ec24b880c13441f413c5d1871fbf5c3a 
> 
> Diff: https://reviews.apache.org/r/47486/diff/
> 
> 
> Testing
> ---
> 
> Windows: build/run
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47499: Make Mesos site responsive.

[haosdent huang]

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




site/source/assets/css/main.css 


Use `max-width` here seems better.


- haosdent huang


On May 18, 2016, 12:24 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47499/
> ---
> 
> (Updated May 18, 2016, 12:24 a.m.)
> 
> 
> Review request for mesos, Tim Anderegg and Vinod Kone.
> 
> 
> Bugs: MESOS-5402
> https://issues.apache.org/jira/browse/MESOS-5402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wrap lines on small screens,
> so the page is more friendly to view on
> mobile.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
> 
> Diff: https://reviews.apache.org/r/47499/diff/
> 
> 
> Testing
> ---
> 
> Manually checked some random pages in different sizes.
> 
> * PC
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/pT87vto4CHMMaG2/old_pc.png)
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/idXPLIM8t0LxRWA/new_pc.png)
> * iPad
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/Wt3g8bknstahXSC/old_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/TBTjlj1xT3YFrOf/new_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/sMct2b54lfCzrAu/old_iPad.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/qp2hj1UCNOfGLIX/new_iPad.png)
> * iPhone
> ![iPhone](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/mCTMOx8FLFxANeo/old_iPhone.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/5hlKDsOKqdJaztw/new_iPhone.png)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47509: Fixed authorization::Request initializings.

[Till Toenshoff]

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

(Updated May 18, 2016, 3:10 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
Vinod Kone.


Summary (updated)
-

Fixed authorization::Request initializings.


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


Repository: mesos


Description
---

Fixes a number of a authorization::Request initializings that were
missing required fields. Also adds a CHECK to the LocalAuthorizer
helping us to catch and prevent these problems in the future.


Diffs
-

  src/authorizer/local/authorizer.cpp e95435327bb3b6f447e814b8657bce8084535346 
  src/common/http.cpp ad6a4b44af3ec847a3a6c0839a88fba18cda5011 
  src/master/http.cpp 6cefcc8b6ddcfe4098521e838bfdc95936fe7476 
  src/master/master.cpp 35b428b0f7dee5954514d8860cfc498271ccf267 
  src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
  src/master/weights_handler.cpp 4bc060fdb015df6658194eef92fe11b14aa15c79 
  src/slave/http.cpp 0fb9b81c4a87250e454e3380c61cf9037454810b 
  src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 

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


Testing
---

make check (OSX)


Thanks,

Till Toenshoff

Review Request 47509: Fixed authorizer::Request initializings.

[Till Toenshoff]

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

Review request for mesos, Adam B, Alexander Rukletsov, Alexander Rojas, and 
Vinod Kone.


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


Repository: mesos


Description
---

Fixes a number of a authorization::Request initializings that were
missing required fields. Also adds a CHECK to the LocalAuthorizer
helping us to catch and prevent these problems in the future.


Diffs
-

  src/authorizer/local/authorizer.cpp e95435327bb3b6f447e814b8657bce8084535346 
  src/common/http.cpp ad6a4b44af3ec847a3a6c0839a88fba18cda5011 
  src/master/http.cpp 6cefcc8b6ddcfe4098521e838bfdc95936fe7476 
  src/master/master.cpp 35b428b0f7dee5954514d8860cfc498271ccf267 
  src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
  src/master/weights_handler.cpp 4bc060fdb015df6658194eef92fe11b14aa15c79 
  src/slave/http.cpp 0fb9b81c4a87250e454e3380c61cf9037454810b 
  src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 

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


Testing
---

make check (OSX)


Thanks,

Till Toenshoff

Re: Review Request 47499: Make Mesos site responsive.

[haosdent huang]

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




site/source/assets/css/main.css (line 14)


We could use max-width here directly to avoid use multiple medias.


- haosdent huang


On May 18, 2016, 12:24 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47499/
> ---
> 
> (Updated May 18, 2016, 12:24 a.m.)
> 
> 
> Review request for mesos, Tim Anderegg and Vinod Kone.
> 
> 
> Bugs: MESOS-5402
> https://issues.apache.org/jira/browse/MESOS-5402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wrap lines on small screens,
> so the page is more friendly to view on
> mobile.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
> 
> Diff: https://reviews.apache.org/r/47499/diff/
> 
> 
> Testing
> ---
> 
> Manually checked some random pages in different sizes.
> 
> * PC
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/pT87vto4CHMMaG2/old_pc.png)
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/idXPLIM8t0LxRWA/new_pc.png)
> * iPad
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/Wt3g8bknstahXSC/old_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/TBTjlj1xT3YFrOf/new_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/sMct2b54lfCzrAu/old_iPad.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/qp2hj1UCNOfGLIX/new_iPad.png)
> * iPhone
> ![iPhone](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/mCTMOx8FLFxANeo/old_iPhone.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/5hlKDsOKqdJaztw/new_iPhone.png)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47499: Make Mesos site responsive.

[haosdent huang]

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



Hi, @janisz May you merge changes from 
https://reviews.apache.org/r/47510/diff/1#index_header . It looks better 
according my test.

- haosdent huang


On May 18, 2016, 12:24 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47499/
> ---
> 
> (Updated May 18, 2016, 12:24 a.m.)
> 
> 
> Review request for mesos, Tim Anderegg and Vinod Kone.
> 
> 
> Bugs: MESOS-5402
> https://issues.apache.org/jira/browse/MESOS-5402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wrap lines on small screens,
> so the page is more friendly to view on
> mobile.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
> 
> Diff: https://reviews.apache.org/r/47499/diff/
> 
> 
> Testing
> ---
> 
> Manually checked some random pages in different sizes.
> 
> * PC
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/pT87vto4CHMMaG2/old_pc.png)
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/idXPLIM8t0LxRWA/new_pc.png)
> * iPad
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/Wt3g8bknstahXSC/old_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/TBTjlj1xT3YFrOf/new_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/sMct2b54lfCzrAu/old_iPad.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/qp2hj1UCNOfGLIX/new_iPad.png)
> * iPhone
> ![iPhone](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/mCTMOx8FLFxANeo/old_iPhone.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/5hlKDsOKqdJaztw/new_iPhone.png)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47510: Adjusted style to make website mobile friendly.

[haosdent huang]

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




site/source/assets/css/main.css (line 24)


Make the image looks better in mobile.



site/source/assets/css/main.css (line 38)


No need padding here because it bring blank in mobile.



site/source/assets/css/main.css 


No need padding here because it bring blank in mobile.



site/source/assets/css/main.css (line 93)


Below contents change to use the responsive navbar of bootstrap.



site/source/index.html.md 


Move it out of row because it's not necessary and make this part looks 
inconsistent with below content in mobile.


- haosdent huang


On May 18, 2016, 2:48 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47510/
> ---
> 
> (Updated May 18, 2016, 2:48 a.m.)
> 
> 
> Review request for mesos, Freddy Ayuso-Henson, Tomasz Janiszewski, Tim 
> Anderegg, and Vinod Kone.
> 
> 
> Bugs: MESOS-3690
> https://issues.apache.org/jira/browse/MESOS-3690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adjusted style to make website mobile friendly.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
>   site/source/layouts/layout.erb 2bf6967faad45644647b732be0fa3c410b9951c5 
> 
> Diff: https://reviews.apache.org/r/47510/diff/
> 
> 
> Testing
> ---
> 
> I pick the necessary changes for mobile friendly from 
> https://github.com/apache/mesos/pull/75 .
> @vinodkone, this should credited to @fayusohenson when submit.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 47510: Adjusted style to make website mobile friendly.

[haosdent huang]

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

Review request for mesos, Freddy Ayuso-Henson, Tomasz Janiszewski, Tim 
Anderegg, and Vinod Kone.


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


Repository: mesos


Description
---

Adjusted style to make website mobile friendly.


Diffs
-

  site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
  site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
  site/source/layouts/layout.erb 2bf6967faad45644647b732be0fa3c410b9951c5 

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


Testing
---

I pick the necessary changes for mobile friendly from 
https://github.com/apache/mesos/pull/75 .
@vinodkone, this should credited to @fayusohenson when submit.


Thanks,

haosdent huang

Re: Review Request 46761: Slave/Agent Rename Phase I - Update terms in WebUI.

[zhou xing]

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

(Updated 五月 18, 2016, 2:16 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, Maxim Khutornenko, and 
Vinod Kone.


Changes
---

rebase code and update the urls according to Vinod's comments


Bugs: mesos-3779
https://issues.apache.org/jira/browse/mesos-3779


Repository: mesos


Description
---

This patch did the following changes in WebUI:
1. Slave/Agent Rename on web page
2. Slave/Agent Rename in web url
3. Slave/Agent Rename in JS class/method/attribute
4. Slave/Agent Rename in comments

For the JSON data returned from server that contains
term 'slave', the change is not included.


Diffs (updated)
-

  CHANGELOG 6bd59d191a8ca59436fa8a1953587119aeb3e256 
  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
  src/webui/master/static/browse.html 04a4600f0c762a2412ddee078ba2c173d595aa8d 
  src/webui/master/static/framework.html 
041513b0e005e8b54ca9723741b21b136ff61ca6 
  src/webui/master/static/home.html 4b201d72f9dfd787133008b8105a225ffb2747aa 
  src/webui/master/static/index.html ec2f5792d21bf7efb479e87be3812b06bfbe98dc 
  src/webui/master/static/js/app.js 543fe9efb9618b311c7f21b7771a251738b01a91 
  src/webui/master/static/js/controllers.js 
8d9021cc89e54ae3a4151ff7f399733f5a7376dd 
  src/webui/master/static/js/services.js 
fa5cc35c1ef0e8ec149ed88852837058ec6ab13c 
  src/webui/master/static/offers.html ec32a649239da48270a1ad1d5bf195326c31ff9d 
  src/webui/master/static/slave.html c908511df85141128599ad5edc40d4b567437822 
  src/webui/master/static/slave_executor.html 
99b23ed9e85011a66bad780fb2d3076e946821a6 
  src/webui/master/static/slave_framework.html 
176e7e9fa7878f31268bd5aa06dfc8789f3e7edd 
  src/webui/master/static/slaves.html 063031771cef8b9f45723869198bad3460591936 

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


Testing
---

cd mesos
./bootstrap
mkdir build
cd build
../configure --prefix=${HOME}/install/mesos
make
make check
make install

start mesos master/agent and run a sample framework, then open browser and goto 
: to check all the strings on web page and url

Tested the following slave relevant urls in browser, and it redirect to 
corresponding agent urls:

  '/slaves', redirectTo: '/agents'

  '/slaves/:agent_id', redirectTo: '/agents/:agent_id'

  '/slaves/:agent_id/frameworks/:framework_id', redirectTo: 
'/agents/:agent_id/frameworks/:framework_id'

  '/slaves/:agent_id/frameworks/:framework_id/executors/:executor_id', 
redirectTo: 
'/agents/:agent_id/frameworks/:framework_id/executors/:executor_id'

  '/slaves/:agent_id/frameworks/:framework_id/executors/:executor_id/browse', 
redirectTo: 
'/agents/:agent_id/frameworks/:framework_id/executors/:executor_id/browse'

  '/slaves/:agent_id/browse', redirectTo: '/agents/:agent_id/browse'


Thanks,

zhou xing

Re: Review Request 47499: Make Mesos site responsive.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 18, 2016, 12:24 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47499/
> ---
> 
> (Updated May 18, 2016, 12:24 a.m.)
> 
> 
> Review request for mesos, Tim Anderegg and Vinod Kone.
> 
> 
> Bugs: MESOS-5402
> https://issues.apache.org/jira/browse/MESOS-5402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wrap lines on small screens,
> so the page is more friendly to view on
> mobile.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
> 
> Diff: https://reviews.apache.org/r/47499/diff/
> 
> 
> Testing
> ---
> 
> Manually checked some random pages in different sizes.
> 
> * PC
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/pT87vto4CHMMaG2/old_pc.png)
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/idXPLIM8t0LxRWA/new_pc.png)
> * iPad
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/Wt3g8bknstahXSC/old_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/TBTjlj1xT3YFrOf/new_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/sMct2b54lfCzrAu/old_iPad.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/qp2hj1UCNOfGLIX/new_iPad.png)
> * iPhone
> ![iPhone](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/mCTMOx8FLFxANeo/old_iPhone.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/5hlKDsOKqdJaztw/new_iPhone.png)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47499: Make Mesos site responsive.

[Tim Anderegg]

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



LGTM. We might want to consider causing the menu to split into 2 columns on 
mobile devices to maximize vertical space, but that's probably a separate 
commit.

- Tim Anderegg


On May 18, 2016, 12:24 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47499/
> ---
> 
> (Updated May 18, 2016, 12:24 a.m.)
> 
> 
> Review request for mesos, Tim Anderegg and Vinod Kone.
> 
> 
> Bugs: MESOS-5402
> https://issues.apache.org/jira/browse/MESOS-5402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wrap lines on small screens,
> so the page is more friendly to view on
> mobile.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
> 
> Diff: https://reviews.apache.org/r/47499/diff/
> 
> 
> Testing
> ---
> 
> Manually checked some random pages in different sizes.
> 
> * PC
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/pT87vto4CHMMaG2/old_pc.png)
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/idXPLIM8t0LxRWA/new_pc.png)
> * iPad
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/Wt3g8bknstahXSC/old_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/TBTjlj1xT3YFrOf/new_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/sMct2b54lfCzrAu/old_iPad.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/qp2hj1UCNOfGLIX/new_iPad.png)
> * iPhone
> ![iPhone](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/mCTMOx8FLFxANeo/old_iPhone.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/5hlKDsOKqdJaztw/new_iPhone.png)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47507: Update CHANGELOG to reflect flags rename.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 18, 2016, 1:04 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47507/
> ---
> 
> (Updated May 18, 2016, 1:04 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3781
> https://issues.apache.org/jira/browse/MESOS-3781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated flags with keyword 'slave' in favor of 'agent'.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 825366d771f3b1bf26cec2ec1cb87619e82f9c8f 
> 
> Diff: https://reviews.apache.org/r/47507/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 46761: Slave/Agent Rename Phase I - Update terms in WebUI.

[Vinod Kone]

> On May 17, 2016, 6:35 p.m., Vinod Kone wrote:
> > src/webui/master/static/js/app.js, lines 37-42
> > 
> >
> > any particular reason why you pulled these? would've been nice to keep 
> > the agent/ and slave/ paths close together.
> > 
> > also, have you tested the redirection? if so, can you update the 
> > "testing section" ?
> 
> zhou xing wrote:
> I moved those lines to let the urls keep alphabetical order, if you think 
> the original order is ok, I will update the patch.
> For the redirection, I have tested them in my browser and it worked, will 
> add this information in the test section

yea, original order is better i think.


- Vinod


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


On May 18, 2016, 1:02 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46761/
> ---
> 
> (Updated May 18, 2016, 1:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, Maxim Khutornenko, 
> and Vinod Kone.
> 
> 
> Bugs: mesos-3779
> https://issues.apache.org/jira/browse/mesos-3779
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch did the following changes in WebUI:
> 1. Slave/Agent Rename on web page
> 2. Slave/Agent Rename in web url
> 3. Slave/Agent Rename in JS class/method/attribute
> 4. Slave/Agent Rename in comments
> 
> For the JSON data returned from server that contains
> term 'slave', the change is not included.
> 
> 
> Diffs
> -
> 
>   CHANGELOG f4b21be457edc0ae4c2b0bec42079b5d99f85118 
>   src/Makefile.am 710e1644e2f0a8e9b87cc997b2211291f4e055fd 
>   src/webui/master/static/browse.html 
> 04a4600f0c762a2412ddee078ba2c173d595aa8d 
>   src/webui/master/static/framework.html 
> 041513b0e005e8b54ca9723741b21b136ff61ca6 
>   src/webui/master/static/home.html 4b201d72f9dfd787133008b8105a225ffb2747aa 
>   src/webui/master/static/index.html ec2f5792d21bf7efb479e87be3812b06bfbe98dc 
>   src/webui/master/static/js/app.js 543fe9efb9618b311c7f21b7771a251738b01a91 
>   src/webui/master/static/js/controllers.js 
> 8d9021cc89e54ae3a4151ff7f399733f5a7376dd 
>   src/webui/master/static/js/services.js 
> fa5cc35c1ef0e8ec149ed88852837058ec6ab13c 
>   src/webui/master/static/offers.html 
> ec32a649239da48270a1ad1d5bf195326c31ff9d 
>   src/webui/master/static/slave.html c908511df85141128599ad5edc40d4b567437822 
>   src/webui/master/static/slave_executor.html 
> 99b23ed9e85011a66bad780fb2d3076e946821a6 
>   src/webui/master/static/slave_framework.html 
> 176e7e9fa7878f31268bd5aa06dfc8789f3e7edd 
>   src/webui/master/static/slaves.html 
> 063031771cef8b9f45723869198bad3460591936 
> 
> Diff: https://reviews.apache.org/r/46761/diff/
> 
> 
> Testing
> ---
> 
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> make check
> make install
> 
> start mesos master/agent and run a sample framework, then open browser and 
> goto : to check all the strings on web page and url
> 
> Tested the following slave relevant urls in browser, and it redirect to 
> corresponding agent urls:
> 
>   '/slaves', redirectTo: '/agents'
> 
>   '/slaves/:agent_id', redirectTo: '/agents/:agent_id'
> 
>   '/slaves/:agent_id/frameworks/:framework_id', redirectTo: 
> '/agents/:agent_id/frameworks/:framework_id'
> 
>   '/slaves/:agent_id/frameworks/:framework_id/executors/:executor_id', 
> redirectTo: 
> '/agents/:agent_id/frameworks/:framework_id/executors/:executor_id'
> 
>   '/slaves/:agent_id/frameworks/:framework_id/executors/:executor_id/browse', 
> redirectTo: 
> '/agents/:agent_id/frameworks/:framework_id/executors/:executor_id/browse'
> 
>   '/slaves/:agent_id/browse', redirectTo: '/agents/:agent_id/browse'
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Review Request 47507: Update CHANGELOG to reflect flags rename.

[Jay Guo]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Deprecated flags with keyword 'slave' in favor of 'agent'.


Diffs
-

  CHANGELOG 825366d771f3b1bf26cec2ec1cb87619e82f9c8f 

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


Testing
---


Thanks,

Jay Guo

Re: Review Request 46761: Slave/Agent Rename Phase I - Update terms in WebUI.

[zhou xing]

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

(Updated 五月 18, 2016, 1:02 a.m.)


Review request for mesos, Benjamin Mahler, Kevin Klues, Maxim Khutornenko, and 
Vinod Kone.


Bugs: mesos-3779
https://issues.apache.org/jira/browse/mesos-3779


Repository: mesos


Description
---

This patch did the following changes in WebUI:
1. Slave/Agent Rename on web page
2. Slave/Agent Rename in web url
3. Slave/Agent Rename in JS class/method/attribute
4. Slave/Agent Rename in comments

For the JSON data returned from server that contains
term 'slave', the change is not included.


Diffs
-

  CHANGELOG f4b21be457edc0ae4c2b0bec42079b5d99f85118 
  src/Makefile.am 710e1644e2f0a8e9b87cc997b2211291f4e055fd 
  src/webui/master/static/browse.html 04a4600f0c762a2412ddee078ba2c173d595aa8d 
  src/webui/master/static/framework.html 
041513b0e005e8b54ca9723741b21b136ff61ca6 
  src/webui/master/static/home.html 4b201d72f9dfd787133008b8105a225ffb2747aa 
  src/webui/master/static/index.html ec2f5792d21bf7efb479e87be3812b06bfbe98dc 
  src/webui/master/static/js/app.js 543fe9efb9618b311c7f21b7771a251738b01a91 
  src/webui/master/static/js/controllers.js 
8d9021cc89e54ae3a4151ff7f399733f5a7376dd 
  src/webui/master/static/js/services.js 
fa5cc35c1ef0e8ec149ed88852837058ec6ab13c 
  src/webui/master/static/offers.html ec32a649239da48270a1ad1d5bf195326c31ff9d 
  src/webui/master/static/slave.html c908511df85141128599ad5edc40d4b567437822 
  src/webui/master/static/slave_executor.html 
99b23ed9e85011a66bad780fb2d3076e946821a6 
  src/webui/master/static/slave_framework.html 
176e7e9fa7878f31268bd5aa06dfc8789f3e7edd 
  src/webui/master/static/slaves.html 063031771cef8b9f45723869198bad3460591936 

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


Testing (updated)
---

cd mesos
./bootstrap
mkdir build
cd build
../configure --prefix=${HOME}/install/mesos
make
make check
make install

start mesos master/agent and run a sample framework, then open browser and goto 
: to check all the strings on web page and url

Tested the following slave relevant urls in browser, and it redirect to 
corresponding agent urls:

  '/slaves', redirectTo: '/agents'

  '/slaves/:agent_id', redirectTo: '/agents/:agent_id'

  '/slaves/:agent_id/frameworks/:framework_id', redirectTo: 
'/agents/:agent_id/frameworks/:framework_id'

  '/slaves/:agent_id/frameworks/:framework_id/executors/:executor_id', 
redirectTo: 
'/agents/:agent_id/frameworks/:framework_id/executors/:executor_id'

  '/slaves/:agent_id/frameworks/:framework_id/executors/:executor_id/browse', 
redirectTo: 
'/agents/:agent_id/frameworks/:framework_id/executors/:executor_id/browse'

  '/slaves/:agent_id/browse', redirectTo: '/agents/:agent_id/browse'


Thanks,

zhou xing

Re: Review Request 46761: Slave/Agent Rename Phase I - Update terms in WebUI.

[zhou xing]

> On 五月 17, 2016, 6:35 p.m., Vinod Kone wrote:
> > src/webui/master/static/js/app.js, lines 37-42
> > 
> >
> > any particular reason why you pulled these? would've been nice to keep 
> > the agent/ and slave/ paths close together.
> > 
> > also, have you tested the redirection? if so, can you update the 
> > "testing section" ?

I moved those lines to let the urls keep alphabetical order, if you think the 
original order is ok, I will update the patch.
For the redirection, I have tested them in my browser and it worked, will add 
this information in the test section


- zhou


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


On 五月 6, 2016, 4:59 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46761/
> ---
> 
> (Updated 五月 6, 2016, 4:59 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, Maxim Khutornenko, 
> and Vinod Kone.
> 
> 
> Bugs: mesos-3779
> https://issues.apache.org/jira/browse/mesos-3779
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch did the following changes in WebUI:
> 1. Slave/Agent Rename on web page
> 2. Slave/Agent Rename in web url
> 3. Slave/Agent Rename in JS class/method/attribute
> 4. Slave/Agent Rename in comments
> 
> For the JSON data returned from server that contains
> term 'slave', the change is not included.
> 
> 
> Diffs
> -
> 
>   CHANGELOG f4b21be457edc0ae4c2b0bec42079b5d99f85118 
>   src/Makefile.am 710e1644e2f0a8e9b87cc997b2211291f4e055fd 
>   src/webui/master/static/browse.html 
> 04a4600f0c762a2412ddee078ba2c173d595aa8d 
>   src/webui/master/static/framework.html 
> 041513b0e005e8b54ca9723741b21b136ff61ca6 
>   src/webui/master/static/home.html 4b201d72f9dfd787133008b8105a225ffb2747aa 
>   src/webui/master/static/index.html ec2f5792d21bf7efb479e87be3812b06bfbe98dc 
>   src/webui/master/static/js/app.js 543fe9efb9618b311c7f21b7771a251738b01a91 
>   src/webui/master/static/js/controllers.js 
> 8d9021cc89e54ae3a4151ff7f399733f5a7376dd 
>   src/webui/master/static/js/services.js 
> fa5cc35c1ef0e8ec149ed88852837058ec6ab13c 
>   src/webui/master/static/offers.html 
> ec32a649239da48270a1ad1d5bf195326c31ff9d 
>   src/webui/master/static/slave.html c908511df85141128599ad5edc40d4b567437822 
>   src/webui/master/static/slave_executor.html 
> 99b23ed9e85011a66bad780fb2d3076e946821a6 
>   src/webui/master/static/slave_framework.html 
> 176e7e9fa7878f31268bd5aa06dfc8789f3e7edd 
>   src/webui/master/static/slaves.html 
> 063031771cef8b9f45723869198bad3460591936 
> 
> Diff: https://reviews.apache.org/r/46761/diff/
> 
> 
> Testing
> ---
> 
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> make check
> make install
> 
> start mesos master/agent and run a sample framework, then open browser and 
> goto : to check all the strings on web page and url
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 47491: Added `environment` field to `Task` protobuf message.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47453, 47490, 47069, 47491]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 17, 2016, 9:27 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47491/
> ---
> 
> (Updated May 17, 2016, 9:27 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-5404
> https://issues.apache.org/jira/browse/MESOS-5404
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> An Authorizer might use environment variables for
> authorization of tasks, so we add it to the \`Task\` protobuf
> message. Note that the master stores \`Task\` (as opposed
> to \`TaskInfo\`) for running and completed tasks.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 887ffe9c5f04e25539a4c6b3d52ce5299c65e8d3 
>   src/common/protobuf_utils.cpp 4f4711d54c471922f1a103310d4d360e41a99870 
> 
> Diff: https://reviews.apache.org/r/47491/diff/
> 
> 
> Testing
> ---
> 
> make check OS-X.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 47505: Updated authorizer.proto Subject, Object and Request.

[Till Toenshoff]

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

Review request for mesos, Adam B, Alexander Rojas, and Vinod Kone.


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


Repository: mesos


Description
---

Makes `value` required for clearity, to make sure that we do not
introduce two ways of expressing `ANY`.


Diffs
-

  include/mesos/authorizer/authorizer.proto 
7d4aa32f42de538864508a0ba481d875917d9ab9 

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


Testing
---

make check (OSX)


Thanks,

Till Toenshoff

Re: Review Request 47499: Make Mesos site responsive.

[Vinod Kone]

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



LGTM. 

@Tim Does this look OK to you?

- Vinod Kone


On May 18, 2016, 12:24 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47499/
> ---
> 
> (Updated May 18, 2016, 12:24 a.m.)
> 
> 
> Review request for mesos, Tim Anderegg and Vinod Kone.
> 
> 
> Bugs: MESOS-5402
> https://issues.apache.org/jira/browse/MESOS-5402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wrap lines on small screens,
> so the page is more friendly to view on
> mobile.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
> 
> Diff: https://reviews.apache.org/r/47499/diff/
> 
> 
> Testing
> ---
> 
> Manually checked some random pages in different sizes.
> 
> * PC
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/pT87vto4CHMMaG2/old_pc.png)
> ![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/idXPLIM8t0LxRWA/new_pc.png)
> * iPad
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/Wt3g8bknstahXSC/old_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/TBTjlj1xT3YFrOf/new_iPad_landscape.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/sMct2b54lfCzrAu/old_iPad.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/qp2hj1UCNOfGLIX/new_iPad.png)
> * iPhone
> ![iPhone](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/mCTMOx8FLFxANeo/old_iPhone.png)
> ![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/5hlKDsOKqdJaztw/new_iPhone.png)
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47499: Make Mesos site responsive.

[Tomasz Janiszewski]

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

(Updated May 18, 2016, 12:24 a.m.)


Review request for mesos, Tim Anderegg and Vinod Kone.


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


Repository: mesos


Description
---

This patch wrap lines on small screens,
so the page is more friendly to view on
mobile.


Diffs (updated)
-

  site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
  site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 

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


Testing (updated)
---

Manually checked some random pages in different sizes.

* PC
![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/pT87vto4CHMMaG2/old_pc.png)
![PC](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/idXPLIM8t0LxRWA/new_pc.png)
* iPad
![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/Wt3g8bknstahXSC/old_iPad_landscape.png)
![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/TBTjlj1xT3YFrOf/new_iPad_landscape.png)
![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/sMct2b54lfCzrAu/old_iPad.png)
![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/qp2hj1UCNOfGLIX/new_iPad.png)
* iPhone
![iPhone](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/mCTMOx8FLFxANeo/old_iPhone.png)
![iPad](https://s3-eu-west-1.amazonaws.com/uploads-eu.hipchat.com/48981/330344/5hlKDsOKqdJaztw/new_iPhone.png)


Thanks,

Tomasz Janiszewski

Re: Review Request 47499: Make Mesos site responsive.

[Tomasz Janiszewski]

> On May 17, 2016, 11:18 p.m., Vinod Kone wrote:
> > Can you attach before and after screenshots to this review?
> > 
> > Also, I'm assuming these changes do not adversely affect the desktop 
> > version?

Posted in testing section. For the first sight diferrences could be hard to 
notice becouse only spacing has changed in PC version. In mobile changes are 
obvious. I have a problem with generating nice screenshot from page with 
vertical scrolls that why old version could be turncated.


> On May 17, 2016, 11:18 p.m., Vinod Kone wrote:
> > site/source/index.html.md, line 6
> > 
> >
> > i'm assuming killing this safe to do?

It'll add padding


- Tomasz


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


On May 17, 2016, 10:48 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47499/
> ---
> 
> (Updated May 17, 2016, 10:48 p.m.)
> 
> 
> Review request for mesos, Tim Anderegg and Vinod Kone.
> 
> 
> Bugs: MESOS-5402
> https://issues.apache.org/jira/browse/MESOS-5402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wrap lines on small screens,
> so the page is more friendly to view on
> mobile.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
> 
> Diff: https://reviews.apache.org/r/47499/diff/
> 
> 
> Testing
> ---
> 
> Manually checked some random pages in different sizes.
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47481: Rewrote os::read() to be friendlier to reading binary data.

[Kevin Klues]

> On May 17, 2016, 8:44 p.m., Jojy Varghese wrote:
> > 3rdparty/stout/include/stout/os/read.hpp, line 113
> > 
> >
> > Does this assume a certain stack size?
> 
> Kevin Klues wrote:
> It would assume a certain stack size, yes. 
> Trust me, I didn't want to go this route personally. I switched to this 
> way based on offline feedback.
> 
> Benjamin Mahler wrote:
> > Trust me, I didn't want to go this route personally. I switched to this 
> way based on offline feedback.
> 
> In the interest of transparency, the initial implementation was directly 
> writing to std::string managed storage. I gave feedback to Kevin to avoid 
> this approach because it's a bit clever and it's not obviously correct 
> compared to writing to a character array and doing the copy: 
> http://stackoverflow.com/a/6700534 . While the string approach is nice 
> because it avoids the extra copy from a character array into the string, 
> `fread` already maintains an internal user-space buffer in order to provide 
> buffered I/O. So, if we really cared about eliminating user space copying we 
> have to use `read` rather than `fread` AFAICT.
> 
> Whether the array is on the heap or the stack is a separate concern IMO, 
> if assuming a particular stack size is a problem, then let's put it on the 
> heap. Curious to hear more thoughts on that however, since we have a number 
> of stack allocated buffers in the code, 1024 bytes or less FWICT.
> 
> Also, why 4096 instead of `BUFSIZ`?
> http://www.cplusplus.com/reference/cstdio/BUFSIZ/
> http://www.cplusplus.com/reference/cstdio/setbuf/
> 
> If we used `BUFSIZ` is there still a concern about stack size assumptions?

I chose 4096 because we had talked about using a PAGE sized buffer. I'm fine 
with any size though.

In general, no matter what size buffer we choose, so long as the buffer is 
stack allocated, there is always a risk of overruning the stack if we are near 
the end of the stack. This isn't just for buffers, it's true for any stack 
allocated variables -- buffers just tend to be rather large stack allocated 
variables, increasing the risk of an overflow.

That said, I'd say the risk is pretty low of overruning your stack on Linux (by 
default the stack can expand up to 8Mb and you can set this higher if you 
want). If you happen to use ulimit to set the max stack size to something 
smaller (or programatically launch threads with smaller stacks), you do 
increase your chances of overrunning the stack (but there was always a non-zero 
cahnce of overruning it anyway).  Either way, Linux will always start with a 
small stack and automatically grow it on demand up to the specified max size 
(page-fault -> allocate new stack -> copy old stack -> update stack pointer -> 
restart task).

The bigger concern for me is accidentally writing buggy code that overruns 
stack allocated buffers in the current stack frame. I've tried to make sure to 
avoid this in this particular function, but I typically like to avoid using 
stack allocated buffers myself.


- Kevin


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


On May 17, 2016, 8:36 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47481/
> ---
> 
> (Updated May 17, 2016, 8:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and switched 
> to 'mcypark'.
> 
> 
> Bugs: MESOS-5398
> https://issues.apache.org/jira/browse/MESOS-5398
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous read() implementation was based on calling getline() to
> read in chunks of data from a file. This is fine for text-based files,
> but is a little strange for binary files.
> 
> The new implementation reads in chunks of raw bytes into a stack
> allocated buffer before copying them into their final location. I
> usually don't like stack allocated buffers because of their potential
> security implications when it comes to stack buffer overflows, but I
> took extra care here to make sure there won't be any.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/read.hpp 
> e1e97c1bcb7493a734fc77721a83c230b1a23724 
> 
> Diff: https://reviews.apache.org/r/47481/diff/
> 
> 
> Testing
> ---
> 
> make check -j
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 47400: Documentation quota authorization changes in 0.29.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47400, 47399]

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

Error:
2016-05-17 23:28:43 URL:https://reviews.apache.org/r/47400/diff/raw/ 
[9744/9744] -> "47400.patch" [1]
error: patch failed: docs/upgrades.md:162
error: docs/upgrades.md: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/13133/console

- Mesos ReviewBot


On May 17, 2016, 7:15 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47400/
> ---
> 
> (Updated May 17, 2016, 7:15 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5155 and MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5155
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation quota authorization changes in 0.29.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 825366d771f3b1bf26cec2ec1cb87619e82f9c8f 
>   docs/authorization.md 0db5c345b3239814b3b9d2e8a87601ff69d0f869 
>   docs/quota.md 797e134605381ae576d9aa93875e0314889ab047 
>   docs/upgrades.md 4f0c86db4c8d51f70487c03b2e75c1a4189b67b9 
> 
> Diff: https://reviews.apache.org/r/47400/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47463: Added documentation for `network/cni` isolator.

[Avinash sridharan]

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

(Updated May 17, 2016, 11:27 p.m.)


Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.


Changes
---

Fixed review comments.


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


Repository: mesos


Description
---

Added documentation for `network/cni` isolator.


Diffs (updated)
-

  docs/cni.md PRE-CREATION 

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


Testing
---

Build the documentation website and verified the rendering.

You can review a rendering of the markdown on my github:
https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md


Thanks,

Avinash sridharan

Re: Review Request 47463: Added documentation for `network/cni` isolator.

[Avinash sridharan]

> On May 17, 2016, 10:17 p.m., Jie Yu wrote:
> > docs/cni.md, line 133
> > 
> >
> > I would mention that nsenter can also be used to achieve the same goal. 
> > One just need to get the pid (which can be get from the bind mount as well).
> > 
> > You don't have to show the commands, just mention it in the text.

Mentioned MESOS-5278 instead of `nsenter`.


- Avinash


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


On May 17, 2016, 11:27 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 11:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4771
> https://issues.apache.org/jira/browse/MESOS-4771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `network/cni` isolator.
> 
> 
> Diffs
> -
> 
>   docs/cni.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47463/diff/
> 
> 
> Testing
> ---
> 
> Build the documentation website and verified the rendering.
> 
> You can review a rendering of the markdown on my github:
> https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 47499: Make Mesos site responsive.

[Vinod Kone]

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



Can you attach before and after screenshots to this review?

Also, I'm assuming these changes do not adversely affect the desktop version?


site/source/index.html.md 


i'm assuming killing this safe to do?


- Vinod Kone


On May 17, 2016, 10:48 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47499/
> ---
> 
> (Updated May 17, 2016, 10:48 p.m.)
> 
> 
> Review request for mesos, Tim Anderegg and Vinod Kone.
> 
> 
> Bugs: MESOS-5402
> https://issues.apache.org/jira/browse/MESOS-5402
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch wrap lines on small screens,
> so the page is more friendly to view on
> mobile.
> 
> 
> Diffs
> -
> 
>   site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
>   site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 
> 
> Diff: https://reviews.apache.org/r/47499/diff/
> 
> 
> Testing
> ---
> 
> Manually checked some random pages in different sizes.
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 45958: Updated protobuf Resource to mark the resource as shareable.

[Anindya Sinha]

> On April 18, 2016, 6:47 a.m., Jiang Yan Xu wrote:
> > include/mesos/mesos.proto, line 762
> > 
> >
> > How about `optional SharedInfo shared`?
> > 
> > [shareable](http://www.merriam-webster.com/dictionary/shareable): 
> > capable of being shared
> > 
> > The following sentences read pretty clear to me.
> > - Some resource types in Mesos are shareable.
> > - Currently only persistent volumes are shareable. (This has nothing to 
> > do with whether `SHARE` operation has been applied, just that this type of 
> > resource can be made shared.)
> > - The `SHARE` operation makes a nonshared resource **shared**. 
> > - The `UNSHARE` operation makes **shared** resource nonshared.
> > - `SharedInfo` is currently empty but in the future we may add policies 
> > around how this resource should be **shared**.
> > 
> > Plus we can compare this with `shared_ptr` which is semantically very 
> > similar.
> > 
> > If we agree to this please also change the use of these words elsewhere 
> > appropriately.
> 
> Anindya Sinha wrote:
> I think ShareInfo seems fine to me. However, I think if there is a strong 
> opinion regarding this, I think Shareable is better simply because it 
> describes the resource (ie. adjective) and is on the same principles as 
> Revocable (as pointed by Guangya Liu).
> However, I stringly believe ShareInfo should be fine.
> 
> Guangya Liu wrote:
> In my understaind, I think that this is similar with `RevocableInfo` as 
> following:
> 
>   message RevocableInfo {}
> 
>   // If this is set, the resources are revocable, i.e., any tasks or
>   // executors launched using these resources could get preempted or
>   // throttled at any time. This could be used by frameworks to run
>   // best effort tasks that do not need strict uptime or performance
>   // guarantees. Note that if this is set, 'disk' or 'reservation'
>   // cannot be set.
>   optional RevocableInfo revocable = 9;
> 
> Agree with Anindya, using the concept of `Shareable` will have same 
> principal with `Revocable`.
> 
> Jiang Yan Xu wrote:
> `shared` and `shareable` are both adjectives, `share` is a verb.
> 
> `shareable` and `shared` have similar meanings except that former 
> emphasizes on **capability** and the latter on the **state**: operation SHARE 
> marks a resource shared just like RESERVE marks a resource reserved 
> (`Resources::isReserved()`), even though in protobuf it's captured by a noun 
> `reservation`.
> 
> Besides IMO **shared** describes the **status** better, my concerns is 
> that in the future we'll need APIs for determining if some resources **are 
> capable of being shared** (`Resources::isShareable(...)`) and we lose the 
> word to conveniently describe it if we use it for something else now. (If 
> this was a one off thing we wouldn't even call it shared resources, just 
> shared volumes).
> 
> My apologies that dicussions around this should have been captured 
> earlier in the design phase but since it's a public API I think we should be 
> more rigorous otherwise it requires deprecation and carefully orchestrated 
> upgrades, etc. to change it in the future.
> 
> Thoughts?

I am not convinced that `shared` is better than `shareable`. However, I do not 
think that it is a great use of time to discuss this further. So, I shall 
change the protobuf field to `shared`. Sounds ok?


- Anindya


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


On April 29, 2016, 12:15 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45958/
> ---
> 
> (Updated April 29, 2016, 12:15 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4892
> https://issues.apache.org/jira/browse/MESOS-4892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added ShareInfo in Resource protobuf to allow for sharing of resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 9a180304996895e2e003085690a7dff9ec561e9c 
>   include/mesos/v1/mesos.proto 44b4f8a059f9dfdcbf02f0c30c1b859898c2e617 
> 
> Diff: https://reviews.apache.org/r/45958/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Review Request 47499: Make Mesos site responsive.

[Tomasz Janiszewski]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This patch wrap lines on small screens,
so the page is more friendly to view on
mobile.


Diffs
-

  site/source/assets/css/main.css 73ca8946091a638a359b1d769961879a283fa099 
  site/source/index.html.md 7b4bdaee61687f487423c1d90a674c78fdf002a4 

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


Testing
---

Manually checked some random pages in different sizes.


Thanks,

Tomasz Janiszewski

Re: Review Request 47489: Windows: Symplified `os::exists`.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47489, 42945]

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

Error:
2016-05-17 22:41:39 URL:https://reviews.apache.org/r/42945/diff/raw/ 
[13298/13298] -> "42945.patch" [1]
error: patch failed: configure.ac:116
error: configure.ac: patch does not apply
error: patch failed: src/Makefile.am:138
error: src/Makefile.am: patch does not apply
error: patch failed: src/master/master.cpp:416
error: src/master/master.cpp: patch does not apply
error: patch failed: src/slave/slave.cpp:805
error: src/slave/slave.cpp: patch does not apply
error: patch failed: src/tests/fault_tolerance_tests.cpp:135
error: src/tests/fault_tolerance_tests.cpp: patch does not apply
error: patch failed: src/tests/mesos.cpp:96
error: src/tests/mesos.cpp: patch does not apply
error: patch failed: src/tests/script.cpp:109
error: src/tests/script.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/13132/console

- Mesos ReviewBot


On May 17, 2016, 7:55 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47489/
> ---
> 
> (Updated May 17, 2016, 7:55 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
> Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Symplified `os::exists`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/exists.hpp 
> 423e4a81b4d34460f7f9f073577e11dfa2d2a520 
> 
> Diff: https://reviews.apache.org/r/47489/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47481: Rewrote os::read() to be friendlier to reading binary data.

[Benjamin Mahler]

> On May 17, 2016, 8:44 p.m., Jojy Varghese wrote:
> > 3rdparty/stout/include/stout/os/read.hpp, line 113
> > 
> >
> > Does this assume a certain stack size?
> 
> Kevin Klues wrote:
> It would assume a certain stack size, yes. 
> Trust me, I didn't want to go this route personally. I switched to this 
> way based on offline feedback.

> Trust me, I didn't want to go this route personally. I switched to this way 
> based on offline feedback.

In the interest of transparency, the initial implementation was directly 
writing to std::string managed storage. I gave feedback to Kevin to avoid this 
approach because it's a bit clever and it's not obviously correct compared to 
writing to a character array and doing the copy: 
http://stackoverflow.com/a/6700534 . While the string approach is nice because 
it avoids the extra copy from a character array into the string, `fread` 
already maintains an internal user-space buffer in order to provide buffered 
I/O. So, if we really cared about eliminating user space copying we have to use 
`read` rather than `fread` AFAICT.

Whether the array is on the heap or the stack is a separate concern IMO, if 
assuming a particular stack size is a problem, then let's put it on the heap. 
Curious to hear more thoughts on that however, since we have a number of stack 
allocated buffers in the code, 1024 bytes or less FWICT.

Also, why 4096 instead of `BUFSIZ`?
http://www.cplusplus.com/reference/cstdio/BUFSIZ/
http://www.cplusplus.com/reference/cstdio/setbuf/

If we used `BUFSIZ` is there still a concern about stack size assumptions?


- Benjamin


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


On May 17, 2016, 8:36 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47481/
> ---
> 
> (Updated May 17, 2016, 8:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and switched 
> to 'mcypark'.
> 
> 
> Bugs: MESOS-5398
> https://issues.apache.org/jira/browse/MESOS-5398
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous read() implementation was based on calling getline() to
> read in chunks of data from a file. This is fine for text-based files,
> but is a little strange for binary files.
> 
> The new implementation reads in chunks of raw bytes into a stack
> allocated buffer before copying them into their final location. I
> usually don't like stack allocated buffers because of their potential
> security implications when it comes to stack buffer overflows, but I
> took extra care here to make sure there won't be any.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/read.hpp 
> e1e97c1bcb7493a734fc77721a83c230b1a23724 
> 
> Diff: https://reviews.apache.org/r/47481/diff/
> 
> 
> Testing
> ---
> 
> make check -j
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 47482: Added preliminary support for parsing ELF files in stout.

[Jojy Varghese]

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




3rdparty/stout/include/stout/elf.hpp (line 77)


Dont we have to close the open file in error cases?



3rdparty/stout/include/stout/elf.hpp (line 94)


Maybe break out after finding DYNAMIC section?



3rdparty/stout/include/stout/elf.hpp (line 184)


emplace_back?



3rdparty/stout/include/stout/elf.hpp (line 196)


const?


- Jojy Varghese


On May 17, 2016, 7:17 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47482/
> ---
> 
> (Updated May 17, 2016, 7:17 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5400
> https://issues.apache.org/jira/browse/MESOS-5400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Right now we are able to parse ELF formatted shared libraries and
> extract their canonical SONAME and external library dependencies. In
> the future, we should add support for fully parsing an ELf file for
> easy access to all of its contents.
> 
> The current implementation relies on libelf. We should probably remove
> this dependency in future versions (mostly since the headers for
> libelf are not installed on a standard Linux distribution by default).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/configure.ac ada1e22c72875fe9d557f07e4846128be0bcef13 
>   3rdparty/stout/include/stout/elf.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47482/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 47463: Added documentation for `network/cni` isolator.

[Jie Yu]

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


Fix it, then Ship it!





docs/cni.md (line 2)


one blank line between the paragraph and the subtitle. Please fix all other 
places.



docs/cni.md (line 133)


I would mention that nsenter can also be used to achieve the same goal. One 
just need to get the pid (which can be get from the bind mount as well).

You don't have to show the commands, just mention it in the text.



docs/cni.md (line 176)


I would typically introduce a blank line above in such cases. Please fix 
them all.



docs/cni.md (line 190)


2 lines part between subsections. Please fix them all.



docs/cni.md (line 221)


kill this line.



docs/cni.md (line 223)


Ditto on adding a new line above.


- Jie Yu


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4771
> https://issues.apache.org/jira/browse/MESOS-4771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `network/cni` isolator.
> 
> 
> Diffs
> -
> 
>   docs/cni.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47463/diff/
> 
> 
> Testing
> ---
> 
> Build the documentation website and verified the rendering.
> 
> You can review a rendering of the markdown on my github:
> https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 47492: Windows: Updated the containerizer name to `mesos-containerizer.exe`.

[Joris Van Remoortere]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/containerizer.cpp (lines 132 - 136)


Can we flip the order here? As in use `ifndef`.


- Joris Van Remoortere


On May 17, 2016, 9:33 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47492/
> ---
> 
> (Updated May 17, 2016, 9:33 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Updated the containerizer name to `mesos-containerizer.exe`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
> 
> Diff: https://reviews.apache.org/r/47492/diff/
> 
> 
> Testing
> ---
> 
> Windows: build/run
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47486: Windows: Escaped command line arguments.

[Joris Van Remoortere]

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




3rdparty/libprocess/include/process/windows/subprocess.hpp (lines 129 - 136)


Can we use `strings::replace` here?
Is there a test that covers this? Can we add a JIRA if not?


- Joris Van Remoortere


On May 17, 2016, 7:19 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47486/
> ---
> 
> (Updated May 17, 2016, 7:19 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
> Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The command line for the containerizer has the command encoded
> as JSON. Non escaped quotes are removed during the containerizer
> startup and the JSON processed is invalid.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/windows/subprocess.hpp 
> 551770b9ec24b880c13441f413c5d1871fbf5c3a 
> 
> Diff: https://reviews.apache.org/r/47486/diff/
> 
> 
> Testing
> ---
> 
> Windows: build/run
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47473: Windows: Added logging for `WSTRINGIFY` calls.

[Joris Van Remoortere]

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




src/common/status_utils.hpp (lines 27 - 30)


It's not really a `no-op` right? It's just not implemented?
Can we adjust the warning, and also add an appropriate `TODO` to implement 
this? Please add the JIRA in the `TODO`.


- Joris Van Remoortere


On May 17, 2016, 5:35 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47473/
> ---
> 
> (Updated May 17, 2016, 5:35 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The return codes in Windows are not standardized. The function returns
> an empty string and logs a warning.
> 
> 
> Diffs
> -
> 
>   src/common/status_utils.hpp fa8dbf1958efa13cfd10712a3215dbca693bcff9 
> 
> Diff: https://reviews.apache.org/r/47473/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> Windows: build/run
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47474: Windows: Disabled signal handlers in logging.cpp.

[Joris Van Remoortere]

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


Fix it, then Ship it!





src/logging/logging.cpp (line 202)


Indentation. Missing period `.`. Please put backticks around 
`InstallFailureSignalHandler`. No need for the `()` in 
`InstallFailureSignalHandler()`.



src/logging/logging.cpp (lines 208 - 212)


indentation.


- Joris Van Remoortere


On May 17, 2016, 5:34 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47474/
> ---
> 
> (Updated May 17, 2016, 5:34 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Disabled signal handlers in logging.cpp.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.cpp 20d2f6341bd39fc5d056f1046d258d006fc602e4 
> 
> Diff: https://reviews.apache.org/r/47474/diff/
> 
> 
> Testing
> ---
> 
> Windows: build/run
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Review Request 47492: Windows: Updated the containerizer name to `mesos-containerizer.exe`.

[Daniel Pravat]

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

Review request for mesos, Alex Naparu, Artem Harutyunyan, Joris Van Remoortere, 
and Michael Park.


Repository: mesos


Description
---

Windows: Updated the containerizer name to `mesos-containerizer.exe`.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
75e5a32a3e70ec60a6800e21a621673184ea0956 

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


Testing
---

Windows: build/run


Thanks,

Daniel Pravat

Re: Review Request 47070: Added framework filtering to /state-summary endoint.

[Joerg Schad]

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

(Updated May 17, 2016, 9:28 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Michael Park.


Repository: mesos


Description
---

As the /state-summary endpoint only outputs the frameworks
(but not tasks) it is sufficient to filter unauthorized frameworks
for this endpoint.


Diffs
-

  src/authorizer/local/authorizer.cpp e59c11269670a7ed72b780913971b421ee17f33f 
  src/master/http.cpp 4985f24b70a00116caa4bd0335ea51e55d878d19 
  src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 

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


Testing
---

make check (osx)


Thanks,

Joerg Schad

Re: Review Request 47069: Added `user` field to `Task` protobuf message.

[Joerg Schad]

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

(Updated May 17, 2016, 9:28 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Michael Park.


Repository: mesos


Description (updated)
---

The LocalAuthorizer might use the OS \`user\` for
authorization of tasks, so we add it to the \`Task\` protobuf
message. Note that the master stores \`Task\` (as opposed
to \`TaskInfo\`) for running and completed tasks.


Diffs (updated)
-

  include/mesos/mesos.proto 887ffe9c5f04e25539a4c6b3d52ce5299c65e8d3 
  src/common/protobuf_utils.cpp 4f4711d54c471922f1a103310d4d360e41a99870 

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


Testing
---

tested entire chain.


Thanks,

Joerg Schad

Review Request 47491: Added `environment` field to `Task` protobuf message.

[Joerg Schad]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

An Authorizer might use environment variables for
authorization of tasks, so we add it to the \`Task\` protobuf
message. Note that the master stores \`Task\` (as opposed
to \`TaskInfo\`) for running and completed tasks.


Diffs
-

  include/mesos/mesos.proto 887ffe9c5f04e25539a4c6b3d52ce5299c65e8d3 
  src/common/protobuf_utils.cpp 4f4711d54c471922f1a103310d4d360e41a99870 

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


Testing
---

make check OS-X.


Thanks,

Joerg Schad

Review Request 47490: Moved `Task` to public protobufs.

[Joerg Schad]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

As we need to authorize Tasks we need to include `Task` in
the public protobufs. This move also requires a the json logic to
adapt.


Diffs
-

  include/mesos/mesos.proto 887ffe9c5f04e25539a4c6b3d52ce5299c65e8d3 
  include/mesos/type_utils.hpp 27fa8c9256c24b5ec6f5d259305ea66b5f556673 
  src/common/http.hpp 00294655ec63a91e67ce3de5e8fb15a836ee1a9c 
  src/common/http.cpp ad6a4b44af3ec847a3a6c0839a88fba18cda5011 
  src/common/type_utils.cpp 037c4336276258d671d0b1bf66cdab50b5bf9fb8 
  src/messages/messages.cpp 41dcdb3996ce173fb0a56704053e4b4e03f6dd63 
  src/messages/messages.proto 5593881229f57a11634357f1f85d697f11f4c827 

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


Testing
---

tested entire chain.


Thanks,

Joerg Schad

Review Request 47453: Corrected order of forwards declarations.

[Joerg Schad]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Forwards declarations should be ordered alphabetically.


Diffs
-

  src/common/http.hpp 00294655ec63a91e67ce3de5e8fb15a836ee1a9c 

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


Testing
---

tested entire chain.


Thanks,

Joerg Schad

Re: Review Request 47474: Windows: Disabled signal handlers in logging.cpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47473, 47474]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 17, 2016, 5:34 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47474/
> ---
> 
> (Updated May 17, 2016, 5:34 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Disabled signal handlers in logging.cpp.
> 
> 
> Diffs
> -
> 
>   src/logging/logging.cpp 20d2f6341bd39fc5d056f1046d258d006fc602e4 
> 
> Diff: https://reviews.apache.org/r/47474/diff/
> 
> 
> Testing
> ---
> 
> Windows: build/run
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47399: New update_quotas ACL for both set and remove cases.

[Zhitao Li]

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

(Updated May 17, 2016, 9:11 p.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Update test done.


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


Repository: mesos


Description
---

New update_quotas ACL for both set and remove cases.


Diffs
-

  include/mesos/authorizer/acls.proto b170330b236b83611d8477c0e45e520ef69aed9b 
  include/mesos/authorizer/authorizer.proto 
7d4aa32f42de538864508a0ba481d875917d9ab9 
  src/authorizer/local/authorizer.hpp d15d3a6c9f3d7c432e593cfe78cc48d672848d02 
  src/authorizer/local/authorizer.cpp bd47f4f02238e5052d9ab6caf338a2b51ddb752d 
  src/master/master.hpp 22a1538b8c2ca85cb8316aafe3f9a4476df833fc 
  src/master/quota_handler.cpp 86a2e175a3e1932961682c2a2d0cfe8210ee5fd0 
  src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 
  src/tests/master_quota_tests.cpp 9bfa6ca58d5a92b857e8f2ce5cb835ddf18b44e6 

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


Testing (updated)
---

1. Unit tests;
2. Manually tested cases: authorized and forbidden under both deprecated 
set_quotas/remove_quotas and new update_quotas, as well as the case that 
specifying both triggers master crash.


Thanks,

Zhitao Li

Re: Review Request 47399: New update_quotas ACL for both set and remove cases.

[Zhitao Li]

> On May 17, 2016, 8:37 p.m., Alexander Rukletsov wrote:
> > Looking good. Have you tested manually the upgrade path?

Yes, I've test both authorized and forbidden case.


- Zhitao


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


On May 17, 2016, 6:26 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47399/
> ---
> 
> (Updated May 17, 2016, 6:26 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5155
> https://issues.apache.org/jira/browse/MESOS-5155
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New update_quotas ACL for both set and remove cases.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> b170330b236b83611d8477c0e45e520ef69aed9b 
>   include/mesos/authorizer/authorizer.proto 
> 7d4aa32f42de538864508a0ba481d875917d9ab9 
>   src/authorizer/local/authorizer.hpp 
> d15d3a6c9f3d7c432e593cfe78cc48d672848d02 
>   src/authorizer/local/authorizer.cpp 
> bd47f4f02238e5052d9ab6caf338a2b51ddb752d 
>   src/master/master.hpp 22a1538b8c2ca85cb8316aafe3f9a4476df833fc 
>   src/master/quota_handler.cpp 86a2e175a3e1932961682c2a2d0cfe8210ee5fd0 
>   src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 
>   src/tests/master_quota_tests.cpp 9bfa6ca58d5a92b857e8f2ce5cb835ddf18b44e6 
> 
> Diff: https://reviews.apache.org/r/47399/diff/
> 
> 
> Testing
> ---
> 
> Unit tests.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47481: Rewrote os::read() to be friendlier to reading binary data.

[Kevin Klues]

> On May 17, 2016, 8:44 p.m., Jojy Varghese wrote:
> > 3rdparty/stout/include/stout/os/read.hpp, line 113
> > 
> >
> > Does this assume a certain stack size?

It would assume a certain stack size, yes. 
Trust me, I didn't want to go this route personally. I switched to this way 
based on offline feedback.


- Kevin


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


On May 17, 2016, 8:36 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47481/
> ---
> 
> (Updated May 17, 2016, 8:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and switched 
> to 'mcypark'.
> 
> 
> Bugs: MESOS-5398
> https://issues.apache.org/jira/browse/MESOS-5398
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous read() implementation was based on calling getline() to
> read in chunks of data from a file. This is fine for text-based files,
> but is a little strange for binary files.
> 
> The new implementation reads in chunks of raw bytes into a stack
> allocated buffer before copying them into their final location. I
> usually don't like stack allocated buffers because of their potential
> security implications when it comes to stack buffer overflows, but I
> took extra care here to make sure there won't be any.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/read.hpp 
> e1e97c1bcb7493a734fc77721a83c230b1a23724 
> 
> Diff: https://reviews.apache.org/r/47481/diff/
> 
> 
> Testing
> ---
> 
> make check -j
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 47069: Added `user` field to `Task` protobuf message.

[Joerg Schad]

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

(Updated May 17, 2016, 8:49 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Michael Park.


Repository: mesos


Description (updated)
---

The LocalAuthorizer will use the OS `user` for
authorization of tasks we add it to the `Task` protobuf
message. Note that the master stores `Task` (as opposed
to `TaskInfo`) for running and completed tasks.


Diffs
-

  src/common/protobuf_utils.cpp 4f4711d54c471922f1a103310d4d360e41a99870 
  src/messages/messages.proto e0f1fca92d3ea8c29c095da31653c317873a934c 

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


Testing (updated)
---

tested entire chain.


Thanks,

Joerg Schad

Re: Review Request 47481: Rewrote os::read() to be friendlier to reading binary data.

[Jojy Varghese]

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




3rdparty/stout/include/stout/os/read.hpp (line 113)


Does this assume a certain stack size?


- Jojy Varghese


On May 17, 2016, 8:36 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47481/
> ---
> 
> (Updated May 17, 2016, 8:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and switched 
> to 'mcypark'.
> 
> 
> Bugs: MESOS-5398
> https://issues.apache.org/jira/browse/MESOS-5398
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous read() implementation was based on calling getline() to
> read in chunks of data from a file. This is fine for text-based files,
> but is a little strange for binary files.
> 
> The new implementation reads in chunks of raw bytes into a stack
> allocated buffer before copying them into their final location. I
> usually don't like stack allocated buffers because of their potential
> security implications when it comes to stack buffer overflows, but I
> took extra care here to make sure there won't be any.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/read.hpp 
> e1e97c1bcb7493a734fc77721a83c230b1a23724 
> 
> Diff: https://reviews.apache.org/r/47481/diff/
> 
> 
> Testing
> ---
> 
> make check -j
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 47399: New update_quotas ACL for both set and remove cases.

[Alexander Rukletsov]

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



Looking good. Have you tested manually the upgrade path?

- Alexander Rukletsov


On May 17, 2016, 6:26 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47399/
> ---
> 
> (Updated May 17, 2016, 6:26 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5155
> https://issues.apache.org/jira/browse/MESOS-5155
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New update_quotas ACL for both set and remove cases.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> b170330b236b83611d8477c0e45e520ef69aed9b 
>   include/mesos/authorizer/authorizer.proto 
> 7d4aa32f42de538864508a0ba481d875917d9ab9 
>   src/authorizer/local/authorizer.hpp 
> d15d3a6c9f3d7c432e593cfe78cc48d672848d02 
>   src/authorizer/local/authorizer.cpp 
> bd47f4f02238e5052d9ab6caf338a2b51ddb752d 
>   src/master/master.hpp 22a1538b8c2ca85cb8316aafe3f9a4476df833fc 
>   src/master/quota_handler.cpp 86a2e175a3e1932961682c2a2d0cfe8210ee5fd0 
>   src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 
>   src/tests/master_quota_tests.cpp 9bfa6ca58d5a92b857e8f2ce5cb835ddf18b44e6 
> 
> Diff: https://reviews.apache.org/r/47399/diff/
> 
> 
> Testing
> ---
> 
> Unit tests.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47481: Rewrote os::read() to be friendlier to reading binary data.

[Kevin Klues]

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

(Updated May 17, 2016, 8:36 p.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and switched 
to 'mcypark'.


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


Repository: mesos


Description (updated)
---

The previous read() implementation was based on calling getline() to
read in chunks of data from a file. This is fine for text-based files,
but is a little strange for binary files.

The new implementation reads in chunks of raw bytes into a stack
allocated buffer before copying them into their final location. I
usually don't like stack allocated buffers because of their potential
security implications when it comes to stack buffer overflows, but I
took extra care here to make sure there won't be any.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/read.hpp 
e1e97c1bcb7493a734fc77721a83c230b1a23724 

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


Testing
---

make check -j


Thanks,

Kevin Klues

Re: Review Request 47400: Documentation quota authorization changes in 0.29.

[Alexander Rukletsov]

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



Looks good to me. I'll commit https://reviews.apache.org/r/46501/ first 
(tomorrow), will ask you to rebase this and do a final pass afterwards. Thanks 
for the great job!


docs/authorization.md (line 359)


s/retrive/query x2
s/retrieve/query



docs/quota.md (line 159)


s/retrieve/query



docs/quota.md (line 189)


s/retrieve/query



docs/upgrades.md (line 169)


Let's mention `UPDATE_QUOTA_WITH_ROLE` as a substitute for deprecated 
actions. Also, please mentions authz actions first and ACLs second, since the 
latter are only for local authorizer.


- Alexander Rukletsov


On May 17, 2016, 7:15 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47400/
> ---
> 
> (Updated May 17, 2016, 7:15 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5155 and MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5155
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documentation quota authorization changes in 0.29.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 825366d771f3b1bf26cec2ec1cb87619e82f9c8f 
>   docs/authorization.md 0db5c345b3239814b3b9d2e8a87601ff69d0f869 
>   docs/quota.md 797e134605381ae576d9aa93875e0314889ab047 
>   docs/upgrades.md 4f0c86db4c8d51f70487c03b2e75c1a4189b67b9 
> 
> Diff: https://reviews.apache.org/r/47400/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47481: Rewrote os::read() to be friendlier to reading binary data.

[Kevin Klues]

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

(Updated May 17, 2016, 8:34 p.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and switched 
to 'mcypark'.


Changes
---

Changed the logic in the read() call to copy from a stack allocated buffer 
instead of writing to the string directly. I usually don't like stack allocated 
buffers because of their potential security implications when it comes to stack 
buffer overflows, but I took extra care here to make sure there won't be any :).


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


Repository: mesos


Description
---

The previous read() implementation was based on calling getline() to
read in chunks of data from a file. This is fine for text-based files,
but is a little strange for binary files.

The new implementation reads in chunks of raw bytes in exponentially
increasing chunk sizes, and works well for both text files and binary
files alike.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/read.hpp 
e1e97c1bcb7493a734fc77721a83c230b1a23724 

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


Testing (updated)
---

make check -j


Thanks,

Kevin Klues

Re: Review Request 46501: Updated authorization.md to reflect current changes.

[Alexander Rukletsov]

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



Please wrap paragraphs at 80 chars.

- Alexander Rukletsov


On May 17, 2016, 1:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46501/
> ---
> 
> (Updated May 17, 2016, 1:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-4785
> https://issues.apache.org/jira/browse/MESOS-4785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The API of the authorization has been changing constantly over the
> last few versions. This patch attempts to update the documentation to
> the those changes into account.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 0db5c345b3239814b3b9d2e8a87601ff69d0f869 
> 
> Diff: https://reviews.apache.org/r/46501/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 47472: Windows: Added support for `fetcher.cpp`.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47472, 47471, 47470, 47469, 47468, 47412, 47411, 47410, 
47409, 47404, 47403, 47391, 47390, 47389, 47388, 47387, 47386, 47169, 47168, 
41632, 47054, 47221, 47053, 47052]

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

Error:
2016-05-17 20:09:51 URL:https://reviews.apache.org/r/47471/diff/raw/ 
[5804/5804] -> "47471.patch" [1]
error: patch failed: src/exec/exec.cpp:595
error: src/exec/exec.cpp: patch does not apply
error: patch failed: src/executor/executor.cpp:154
error: src/executor/executor.cpp: patch does not apply
error: patch failed: src/launcher/fetcher.cpp:443
error: src/launcher/fetcher.cpp: patch does not apply

Full log: https://builds.apache.org/job/mesos-reviewbot/13129/console

- Mesos ReviewBot


On May 17, 2016, 4:02 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47472/
> ---
> 
> (Updated May 17, 2016, 4:02 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-3618
> https://issues.apache.org/jira/browse/MESOS-3618
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added support for `fetcher.cpp`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/fetcher.cpp 
> 176d8863d1becd8864218a0012ab45c614f0ad77 
> 
> Diff: https://reviews.apache.org/r/47472/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47463: Added documentation for `network/cni` isolator.

[Avinash sridharan]

> On May 17, 2016, 7:47 p.m., Michael Cambria wrote:
> > docs/cni.md, line 54
> > 
> >
> > This reads like the plugin is always responsible for creating veth 
> > pairs.  Is this always the case or just applicable for e.g. bridge network 
> > types?

The plugin is always responsible for creating the `veth` pair.


- Avinash


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


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4771
> https://issues.apache.org/jira/browse/MESOS-4771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `network/cni` isolator.
> 
> 
> Diffs
> -
> 
>   docs/cni.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47463/diff/
> 
> 
> Testing
> ---
> 
> Build the documentation website and verified the rendering.
> 
> You can review a rendering of the markdown on my github:
> https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 47489: Windows: Symplified `os::exists`.

[Alex Naparu]

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


Ship it!




Ship It!

- Alex Naparu


On May 17, 2016, 7:55 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47489/
> ---
> 
> (Updated May 17, 2016, 7:55 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
> Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Symplified `os::exists`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/exists.hpp 
> 423e4a81b4d34460f7f9f073577e11dfa2d2a520 
> 
> Diff: https://reviews.apache.org/r/47489/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47363: Introduced a driver to v1 executor shim/adapter.

[Anand Mazumdar]

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

(Updated May 17, 2016, 7:57 p.m.)


Review request for mesos, Joris Van Remoortere and Vinod Kone.


Changes
---

Review comments


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


Repository: mesos


Description
---

This change adds a driver to v1 executor shim/adapter. This can
be used by the command executor/docker executor to toggle
between using the new/old API instead of duplicating the
code like we did with `http_command_executor.cpp`.


Diffs (updated)
-

  include/mesos/v1/executor.hpp adca287be3bb88c8b3298705740cb6bcbb3a09f0 
  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
  src/executor/v0_v1executor.hpp PRE-CREATION 
  src/executor/v0_v1executor.cpp PRE-CREATION 

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


Testing
---

make check (Later in the chain, we make the command executor use this interface)
So the existing tests should be able to test this out.


Thanks,

Anand Mazumdar

Review Request 47489: Windows: Symplified `os::exists`.

[Daniel Pravat]

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

Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
Van Remoortere, and Michael Park.


Repository: mesos


Description
---

Windows: Symplified `os::exists`.


Diffs
-

  3rdparty/stout/include/stout/os/windows/exists.hpp 
423e4a81b4d34460f7f9f073577e11dfa2d2a520 

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


Testing
---


Thanks,

Daniel Pravat

Re: Review Request 47463: Added documentation for `network/cni` isolator.

[Michael Cambria]

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




docs/cni.md (line 54)


This reads like the plugin is always responsible for creating veth pairs.  
Is this always the case or just applicable for e.g. bridge network types?


- Michael Cambria


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4771
> https://issues.apache.org/jira/browse/MESOS-4771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `network/cni` isolator.
> 
> 
> Diffs
> -
> 
>   docs/cni.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47463/diff/
> 
> 
> Testing
> ---
> 
> Build the documentation website and verified the rendering.
> 
> You can review a rendering of the markdown on my github:
> https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 47463: Added documentation for `network/cni` isolator.

[Michael Cambria]

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




docs/cni.md (line 23)


macvlan network is still listed in the Networking Recipes, but this section 
has been reoved.


- Michael Cambria


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4771
> https://issues.apache.org/jira/browse/MESOS-4771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `network/cni` isolator.
> 
> 
> Diffs
> -
> 
>   docs/cni.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47463/diff/
> 
> 
> Testing
> ---
> 
> Build the documentation website and verified the rendering.
> 
> You can review a rendering of the markdown on my github:
> https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 46501: Updated authorization.md to reflect current changes.

[Greg Mann]

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


Fix it, then Ship it!




Hey Alexander, the text looks great to me, thanks for your careful editing! I 
have just one small comment below. Also, I was going to view this in the 
mesos-website-container, but when I tried to apply the patch I got a conflict, 
so it seems to need some rebasing?


docs/authorization.md (line 40)


s/ALC/ACL/


- Greg Mann


On May 17, 2016, 1:11 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46501/
> ---
> 
> (Updated May 17, 2016, 1:11 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-4785
> https://issues.apache.org/jira/browse/MESOS-4785
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The API of the authorization has been changing constantly over the
> last few versions. This patch attempts to update the documentation to
> the those changes into account.
> 
> 
> Diffs
> -
> 
>   docs/authorization.md 0db5c345b3239814b3b9d2e8a87601ff69d0f869 
> 
> Diff: https://reviews.apache.org/r/46501/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 47442: Windows: Enable `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` on job objects.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47442]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 17, 2016, 3:40 p.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47442/
> ---
> 
> (Updated May 17, 2016, 3:40 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
> Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Enable `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` on job objects.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 261768eace6ab09956f4a80e1ec5dba988d831e1 
> 
> Diff: https://reviews.apache.org/r/47442/diff/
> 
> 
> Testing
> ---
> 
> Windows build/test
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 47463: Added documentation for `network/cni` isolator.

[Michael Cambria]

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



Networking Recipes still lists macvlan as a recipe, prior edit removed this 
section

- Michael Cambria


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4771
> https://issues.apache.org/jira/browse/MESOS-4771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `network/cni` isolator.
> 
> 
> Diffs
> -
> 
>   docs/cni.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47463/diff/
> 
> 
> Testing
> ---
> 
> Build the documentation website and verified the rendering.
> 
> You can review a rendering of the markdown on my github:
> https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 47431: Renamed `HttpCommandExecutor` to `CommandExecutor`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 16, 2016, 11:49 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47431/
> ---
> 
> (Updated May 16, 2016, 11:49 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5302
> https://issues.apache.org/jira/browse/MESOS-5302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 7d111e668e0a139a98bdeb959997843180b40452 
> 
> Diff: https://reviews.apache.org/r/47431/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 47364: Set env variable used to toggle executor implementation.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 13, 2016, 10:34 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47364/
> ---
> 
> (Updated May 13, 2016, 10:34 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5063
> https://issues.apache.org/jira/browse/MESOS-5063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the environment variable that is set by
> the agent allowing the command executor to toggle between
> implementations.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> d0cae79834e451594d7675f00c5f7d2d2cd3a264 
> 
> Diff: https://reviews.apache.org/r/47364/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 47365: Moved code from HTTP command executor to command executor.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On May 16, 2016, 11:50 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47365/
> ---
> 
> (Updated May 16, 2016, 11:50 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5302
> https://issues.apache.org/jira/browse/MESOS-5302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a blatant copy of `src/launcher/http_command_executor.cpp`.
> This change makes reviewing code later in the chain easier.
> Later in the chain, we would make this use the Shim/Adapter
> to toggle between using the driver or the v1 API via an
> environment variable. The existing `http_command_executor.cpp` 
> would be deleted.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 7d111e668e0a139a98bdeb959997843180b40452 
> 
> Diff: https://reviews.apache.org/r/47365/diff/
> 
> 
> Testing
> ---
> 
> make check
> (this should not be committed on its own)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 46892: Postponed closing std streams of the docker task until after reaping.

[Alexander Rukletsov]

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




src/docker/executor.cpp (line 308)


`.await()` blocks the execution and synchronously wait for docker to 
terminate. I'd expect that during this period the docker executor won't be 
responsive to anything, like escalating kills. Moreover, I'd expect that 
`TASK_KILLING` won't be sent by the driver since we are blocked in the callback.

If I understand your intention correctly, you want to discard `run` only 
after the container terminates. This is what `reaped()` callbakc is for: it is 
invoked when the container terminates, i.e. `stop` has signaled.

My only concern about discarding `run` in `reaped()` is for the case when 
docker container is starting but has not yet started. I would like us to test 
this case.



src/docker/executor.cpp (line 358)


Why this change?


- Alexander Rukletsov


On May 17, 2016, 6:15 p.m., Martin Bydzovsky wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46892/
> ---
> 
> (Updated May 17, 2016, 6:15 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-4279
> https://issues.apache.org/jira/browse/MESOS-4279
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 579905f6c2ece7db9c853abf481541fdbc3116b7 
> 
> Diff: https://reviews.apache.org/r/46892/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Bydzovsky
> 
>

Re: Review Request 47363: Introduced a driver to v1 executor shim/adapter.

[Vinod Kone]

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


Fix it, then Ship it!





include/mesos/v1/executor.hpp (line 74)


s/;/ override;/ ?


- Vinod Kone


On May 17, 2016, 3 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47363/
> ---
> 
> (Updated May 17, 2016, 3 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Bugs: MESOS-5302
> https://issues.apache.org/jira/browse/MESOS-5302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a driver to v1 executor shim/adapter. This can
> be used by the command executor/docker executor to toggle
> between using the new/old API instead of duplicating the
> code like we did with `http_command_executor.cpp`.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor.hpp adca287be3bb88c8b3298705740cb6bcbb3a09f0 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
>   src/executor/v0_v1executor.hpp PRE-CREATION 
>   src/executor/v0_v1executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47363/diff/
> 
> 
> Testing
> ---
> 
> make check (Later in the chain, we make the command executor use this 
> interface)
> So the existing tests should be able to test this out.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 47486: Windows: Escaped command line arguments.

[Daniel Pravat]

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

Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
Van Remoortere, and Michael Park.


Repository: mesos


Description
---

The command line for the containerizer has the command encoded
as JSON. Non escaped quotes are removed during the containerizer
startup and the JSON processed is invalid.


Diffs
-

  3rdparty/libprocess/include/process/windows/subprocess.hpp 
551770b9ec24b880c13441f413c5d1871fbf5c3a 

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


Testing
---

Windows: build/run


Thanks,

Daniel Pravat

Review Request 47483: Added -lelf to default dynamic libraries for libprocess on Linux.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Added -lelf to default dynamic libraries for libprocess on Linux.


Diffs
-

  3rdparty/libprocess/configure.ac c8fcd35241c10829e7b2fa582491898589f0576f 

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


Testing
---


Thanks,

Kevin Klues

Review Request 47484: Added -lelf to default dynamic libraries on Linux.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Added -lelf to default dynamic libraries on Linux.


Diffs
-

  configure.ac 63ea028fa89ba8e164e98226fc7ddcffd8b045c8 

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


Testing
---


Thanks,

Kevin Klues

Review Request 47482: Added preliminary support for parsing ELF files in stout.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Right now we are able to parse ELF formatted shared libraries and
extract their canonical SONAME and external library dependencies. In
the future, we should add support for fully parsing an ELf file for
easy access to all of its contents.

The current implementation relies on libelf. We should probably remove
this dependency in future versions (mostly since the headers for
libelf are not installed on a standard Linux distribution by default).


Diffs
-

  3rdparty/stout/configure.ac ada1e22c72875fe9d557f07e4846128be0bcef13 
  3rdparty/stout/include/stout/elf.hpp PRE-CREATION 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 47400: Documentation quota authorization changes in 0.29.

[Zhitao Li]

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

(Updated May 17, 2016, 7:15 p.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Rebase, changelog, and review comments.


Bugs: MESOS-5155 and MESOS-5336
https://issues.apache.org/jira/browse/MESOS-5155
https://issues.apache.org/jira/browse/MESOS-5336


Repository: mesos


Description
---

Documentation quota authorization changes in 0.29.


Diffs (updated)
-

  CHANGELOG 825366d771f3b1bf26cec2ec1cb87619e82f9c8f 
  docs/authorization.md 0db5c345b3239814b3b9d2e8a87601ff69d0f869 
  docs/quota.md 797e134605381ae576d9aa93875e0314889ab047 
  docs/upgrades.md 4f0c86db4c8d51f70487c03b2e75c1a4189b67b9 

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


Testing
---


Thanks,

Zhitao Li

Review Request 47485: Added utility for parsing ld.so.cache on linux.

[Kevin Klues]

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Added utility for parsing ld.so.cache on linux.


Diffs
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
  src/linux/ldcache.hpp PRE-CREATION 
  src/linux/ldcache.cpp PRE-CREATION 
  src/tests/ldcache_tests.cpp PRE-CREATION 

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


Testing
---

GTEST_FILTER="LdcacheTest.Parse" make check -j


Thanks,

Kevin Klues

Review Request 47481: Rewrote os::read() to be friendlier to reading binary data.

[Kevin Klues]

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

Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and switched 
to 'mcypark'.


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


Repository: mesos


Description
---

The previous read() implementation was based on calling getline() to
read in chunks of data from a file. This is fine for text-based files,
but is a little strange for binary files.

The new implementation reads in chunks of raw bytes in exponentially
increasing chunk sizes, and works well for both text files and binary
files alike.


Diffs
-

  3rdparty/stout/include/stout/os/read.hpp 
e1e97c1bcb7493a734fc77721a83c230b1a23724 

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


Testing
---


Thanks,

Kevin Klues

Re: Review Request 47463: Added documentation for `network/cni` isolator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47463]

Passed command: export OS='ubuntu:14.04' CONFIGURATION='--verbose' 
COMPILER='gcc' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker_build.sh

- Mesos ReviewBot


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4771
> https://issues.apache.org/jira/browse/MESOS-4771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `network/cni` isolator.
> 
> 
> Diffs
> -
> 
>   docs/cni.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47463/diff/
> 
> 
> Testing
> ---
> 
> Build the documentation website and verified the rendering.
> 
> You can review a rendering of the markdown on my github:
> https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 46761: Slave/Agent Rename Phase I - Update terms in WebUI.

[Vinod Kone]

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




src/webui/master/static/js/app.js (lines 30 - 35)


any particular reason why you pulled these? would've been nice to keep the 
agent/ and slave/ paths close together.

also, have you tested the redirection? if so, can you update the "testing 
section" ?


- Vinod Kone


On May 6, 2016, 4:59 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46761/
> ---
> 
> (Updated May 6, 2016, 4:59 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, Maxim Khutornenko, 
> and Vinod Kone.
> 
> 
> Bugs: mesos-3779
> https://issues.apache.org/jira/browse/mesos-3779
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch did the following changes in WebUI:
> 1. Slave/Agent Rename on web page
> 2. Slave/Agent Rename in web url
> 3. Slave/Agent Rename in JS class/method/attribute
> 4. Slave/Agent Rename in comments
> 
> For the JSON data returned from server that contains
> term 'slave', the change is not included.
> 
> 
> Diffs
> -
> 
>   CHANGELOG f4b21be457edc0ae4c2b0bec42079b5d99f85118 
>   src/Makefile.am 710e1644e2f0a8e9b87cc997b2211291f4e055fd 
>   src/webui/master/static/browse.html 
> 04a4600f0c762a2412ddee078ba2c173d595aa8d 
>   src/webui/master/static/framework.html 
> 041513b0e005e8b54ca9723741b21b136ff61ca6 
>   src/webui/master/static/home.html 4b201d72f9dfd787133008b8105a225ffb2747aa 
>   src/webui/master/static/index.html ec2f5792d21bf7efb479e87be3812b06bfbe98dc 
>   src/webui/master/static/js/app.js 543fe9efb9618b311c7f21b7771a251738b01a91 
>   src/webui/master/static/js/controllers.js 
> 8d9021cc89e54ae3a4151ff7f399733f5a7376dd 
>   src/webui/master/static/js/services.js 
> fa5cc35c1ef0e8ec149ed88852837058ec6ab13c 
>   src/webui/master/static/offers.html 
> ec32a649239da48270a1ad1d5bf195326c31ff9d 
>   src/webui/master/static/slave.html c908511df85141128599ad5edc40d4b567437822 
>   src/webui/master/static/slave_executor.html 
> 99b23ed9e85011a66bad780fb2d3076e946821a6 
>   src/webui/master/static/slave_framework.html 
> 176e7e9fa7878f31268bd5aa06dfc8789f3e7edd 
>   src/webui/master/static/slaves.html 
> 063031771cef8b9f45723869198bad3460591936 
> 
> Diff: https://reviews.apache.org/r/46761/diff/
> 
> 
> Testing
> ---
> 
> cd mesos
> ./bootstrap
> mkdir build
> cd build
> ../configure --prefix=${HOME}/install/mesos
> make
> make check
> make install
> 
> start mesos master/agent and run a sample framework, then open browser and 
> goto : to check all the strings on web page and url
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 47478: Fixed broken link for Academic Papers in documentation.

[haosdent huang]

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

(Updated May 17, 2016, 6:27 p.m.)


Review request for mesos, Neil Conway and Vinod Kone.


Changes
---

Address @vinodkone's comment.


Repository: mesos


Description
---

Fixed broken link for Academic Papers in documentation.


Diffs (updated)
-

  docs/home.md dc41fc4479e6c23650cd8ac78dcc4b9161d00721 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 47478: Fixed broken link for Academic Papers in documentation.

[haosdent huang]

> On May 17, 2016, 6:12 p.m., Vinod Kone wrote:
> > docs/home.md, line 94
> > 
> >
> > How about this link?
> > 
> > 
> > https://www.usenix.org/conference/nsdi11/mesos-platform-fine-grained-resource-sharing-data-center

It looks better, just updated.


- haosdent


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


On May 17, 2016, 6:27 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47478/
> ---
> 
> (Updated May 17, 2016, 6:27 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed broken link for Academic Papers in documentation.
> 
> 
> Diffs
> -
> 
>   docs/home.md dc41fc4479e6c23650cd8ac78dcc4b9161d00721 
> 
> Diff: https://reviews.apache.org/r/47478/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47399: New update_quotas ACL for both set and remove cases.

[Zhitao Li]

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

(Updated May 17, 2016, 6:26 p.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Remove "depends on" since it's already landed.


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


Repository: mesos


Description
---

New update_quotas ACL for both set and remove cases.


Diffs
-

  include/mesos/authorizer/acls.proto b170330b236b83611d8477c0e45e520ef69aed9b 
  include/mesos/authorizer/authorizer.proto 
7d4aa32f42de538864508a0ba481d875917d9ab9 
  src/authorizer/local/authorizer.hpp d15d3a6c9f3d7c432e593cfe78cc48d672848d02 
  src/authorizer/local/authorizer.cpp bd47f4f02238e5052d9ab6caf338a2b51ddb752d 
  src/master/master.hpp 22a1538b8c2ca85cb8316aafe3f9a4476df833fc 
  src/master/quota_handler.cpp 86a2e175a3e1932961682c2a2d0cfe8210ee5fd0 
  src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 
  src/tests/master_quota_tests.cpp 9bfa6ca58d5a92b857e8f2ce5cb835ddf18b44e6 

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


Testing
---

Unit tests.


Thanks,

Zhitao Li

Re: Review Request 47399: New update_quotas ACL for both set and remove cases.

[Zhitao Li]

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

(Updated May 17, 2016, 6:25 p.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Rebase, review comments and style fixes.


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


Repository: mesos


Description
---

New update_quotas ACL for both set and remove cases.


Diffs (updated)
-

  include/mesos/authorizer/acls.proto b170330b236b83611d8477c0e45e520ef69aed9b 
  include/mesos/authorizer/authorizer.proto 
7d4aa32f42de538864508a0ba481d875917d9ab9 
  src/authorizer/local/authorizer.hpp d15d3a6c9f3d7c432e593cfe78cc48d672848d02 
  src/authorizer/local/authorizer.cpp bd47f4f02238e5052d9ab6caf338a2b51ddb752d 
  src/master/master.hpp 22a1538b8c2ca85cb8316aafe3f9a4476df833fc 
  src/master/quota_handler.cpp 86a2e175a3e1932961682c2a2d0cfe8210ee5fd0 
  src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 
  src/tests/master_quota_tests.cpp 9bfa6ca58d5a92b857e8f2ce5cb835ddf18b44e6 

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


Testing
---

Unit tests.


Thanks,

Zhitao Li

Re: Review Request 47399: New update_quotas ACL for both set and remove cases.

[Zhitao Li]

> On May 17, 2016, 2:34 p.m., Alexander Rukletsov wrote:
> > src/tests/master_quota_tests.cpp, line 1151
> > 
> >
> > set_quotas?

Sorry for typo. Should be get_quotas.


- Zhitao


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


On May 17, 2016, 6:25 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47399/
> ---
> 
> (Updated May 17, 2016, 6:25 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5155
> https://issues.apache.org/jira/browse/MESOS-5155
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> New update_quotas ACL for both set and remove cases.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> b170330b236b83611d8477c0e45e520ef69aed9b 
>   include/mesos/authorizer/authorizer.proto 
> 7d4aa32f42de538864508a0ba481d875917d9ab9 
>   src/authorizer/local/authorizer.hpp 
> d15d3a6c9f3d7c432e593cfe78cc48d672848d02 
>   src/authorizer/local/authorizer.cpp 
> bd47f4f02238e5052d9ab6caf338a2b51ddb752d 
>   src/master/master.hpp 22a1538b8c2ca85cb8316aafe3f9a4476df833fc 
>   src/master/quota_handler.cpp 86a2e175a3e1932961682c2a2d0cfe8210ee5fd0 
>   src/tests/authorization_tests.cpp f50ac69c32d0551a63391d97f31559ff4f414011 
>   src/tests/master_quota_tests.cpp 9bfa6ca58d5a92b857e8f2ce5cb835ddf18b44e6 
> 
> Diff: https://reviews.apache.org/r/47399/diff/
> 
> 
> Testing
> ---
> 
> Unit tests.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47463: Added documentation for `network/cni` isolator.

[Tomasz Janiszewski]

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




docs/cni.md (line 47)


Missing space before `(`


- Tomasz Janiszewski


On May 17, 2016, 3:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47463/
> ---
> 
> (Updated May 17, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Kevin Klues, Neil Conway, and Qian Zhang.
> 
> 
> Bugs: MESOS-4771
> https://issues.apache.org/jira/browse/MESOS-4771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for `network/cni` isolator.
> 
> 
> Diffs
> -
> 
>   docs/cni.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47463/diff/
> 
> 
> Testing
> ---
> 
> Build the documentation website and verified the rendering.
> 
> You can review a rendering of the markdown on my github:
> https://github.com/asridharan/mesos/blob/MESOS-4771/docs/cni.md
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 46892: Postponed closing std streams of the docker task until after reaping.

[Martin Bydzovsky]

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

(Updated May 17, 2016, 6:15 p.m.)


Review request for mesos and Alexander Rukletsov.


Summary (updated)
-

Postponed closing std streams of the docker task until after reaping.


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


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  src/docker/executor.cpp 579905f6c2ece7db9c853abf481541fdbc3116b7 

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


Testing
---


Thanks,

Martin Bydzovsky